Skip to content

add rustc-demangle assertion on mangled symbol#85534

Merged
bors merged 1 commit intorust-lang:masterfrom
csmoe:demagnle-assert
Aug 29, 2021
Merged

add rustc-demangle assertion on mangled symbol#85534
bors merged 1 commit intorust-lang:masterfrom
csmoe:demagnle-assert

Conversation

@csmoe
Copy link
Contributor

@csmoe csmoe commented May 21, 2021

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 21, 2021
@michaelwoerister
Copy link
Member

michaelwoerister commented May 21, 2021

Thanks, @csmoe! Let's wait and see what the crater run at #85530 yields.

@michaelwoerister
Copy link
Member

michaelwoerister commented May 21, 2021

I'm also wondering if this should be a debug_assert! instead. @Mark-Simulacrum, do you have any idea how much coverage we would get if this is only a debug assertion? Does anybody use a compiler with debug assertions enabled?

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented May 21, 2021

Essentially only rustc developers, I'm not sure we even ship builds with debug asserts on. Do we know what performance impact we're looking at?

@csmoe
Copy link
Contributor Author

csmoe commented May 22, 2021

@michaelwoerister Or a crater_run_assert which checks if we are doing a crater_run through an env variable.

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented May 22, 2021

Please don't try to detect crater inside rustc; if you want to test something on crater but not land it, a try build should be sufficient with it just always on, and we can run that through crater.

@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 11, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 27, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 12, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 31, 2021
@michaelwoerister michaelwoerister added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 12, 2021
@michaelwoerister
Copy link
Member

michaelwoerister commented Aug 12, 2021

I think this is blocked on value mangling support: #87194

Once we have that, we should at least make it a debug assertion. That way we cover everything that the compiler itself uses.

@michaelwoerister
Copy link
Member

michaelwoerister commented Aug 27, 2021

@csmoe, now that #87194 is merged, would you mind turning the check into a debug assertion?

@csmoe
Copy link
Contributor Author

csmoe commented Aug 27, 2021

@michaelwoerister okay :)

michaelwoerister
michaelwoerister approved these changes Aug 27, 2021
View reviewed changes
Copy link
Member

@michaelwoerister michaelwoerister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @csmoe!

@michaelwoerister
Copy link
Member

michaelwoerister commented Aug 27, 2021

@bors r+

@michaelwoerister
Copy link
Member

michaelwoerister commented Aug 27, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 27, 2021

📌 Commit 5eb960c has been approved by michaelwoerister

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 27, 2021
@michaelwoerister
Copy link
Member

michaelwoerister commented Aug 27, 2021

bors doesn't like PRs that have been open for a while :)

@jackh726
Copy link
Member

jackh726 commented Aug 27, 2021

@bors rollup

@GuillaumeGomez GuillaumeGomez mentioned this pull request Aug 29, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2021
@bors
Auto merge of rust-lang#88461 - GuillaumeGomez:rollup-khgu0eb, r=Guil…
daa4dc9 
…laumeGomez

Rollup of 13 pull requests

Successful merges:

 - rust-lang#80543 (Notify when an `I-prioritize` issue is closed or reopened)
 - rust-lang#83251 (Suggestion for call on immutable binding of mutable type)
 - rust-lang#85534 (add rustc-demangle assertion on mangled symbol)
 - rust-lang#88173 (Refactor Markdown length-limited summary implementation)
 - rust-lang#88349 (Add const and static TAIT tests)
 - rust-lang#88357 (add unsized coercion test)
 - rust-lang#88381 (Handle stack_t.ss_sp type change for DragonFlyBSD)
 - rust-lang#88387 (Remove vestigial rustfix tests.)
 - rust-lang#88396 (Bump vulnerable crates)
 - rust-lang#88407 (Fix formatting in release notes from 52a9883)
 - rust-lang#88411 (Remove `Session.if_let_suggestions`)
 - rust-lang#88417 (RELEASES.md: fix broken link)
 - rust-lang#88419 (Fix code blocks color in Ayu theme)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 63cfbf5 into rust-lang:master Aug 29, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@michaelwoerister michaelwoerister michaelwoerister approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@michaelwoerister michaelwoerister

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.56.0

Development

Successfully merging this pull request may close these issues.

9 participants

@csmoe @michaelwoerister @Mark-Simulacrum @bors @jackh726 @crlf0710 @JohnCSimon @rust-highfive @rustbot