Account for identity substituted items in symbol mangling #135261
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred in coverage tests. cc @Zalathar |
|
Is there anything that coverage could potentially do differently to make this less of a hassle? For unused non-generic functions, we want to synthesize the same symbol as if that function had actually been emitted. But for unused generic functions we have a bit more latitude, since the symbol name is fundamentally fake. The biggest constraint is that it has to demangle into something that the user can understand. |
|
Nah, this was just an oversight on my part. I think this is really the only choice we have for unused symbols. |
| // 2. We're mangling an item with identity substs. This seems to only happen | ||
| // when generating coverage, since we try to generate coverage for unused | ||
| // items too, and if something isn't monomorphized then we necessarily don't | ||
| // have anything to substitute the instance with. |
There was a problem hiding this comment.
Note that -Cinstrument-coverage automatically implies -Csymbol-mangling-version=v0, so normally this case shouldn't arise for legacy mangling ...
... except that if you explicitly do -Cinstrument-coverage -Csymbol-mangling-version=legacy -Zunstable-options, you do end up with coverage instrumentation using legacy mangling anyway, with only a compiler warning. So this case can arise after all.
(In other words, I don't think there's anything actionable in this review comment; I just wanted to note my sadness.)
There was a problem hiding this comment.
It's nbd, I'd rather keep these in sync even if the legacy branch is never exercised.
|
@bors r+ |
bors
added
S-waiting-on-bors
|
i added a comment to the ui test @bors r=oli-obk |
This comment has been minimized.
This comment has been minimized.
|
if i had known it would cause ci failures i would have ignored that 😅 gonna check it out locally to fix and then force-puzsh |
lcnr
force-pushed
the
coverage-has-identity-substs
branch
from
91e1e70 to
b85b2c6
Compare
|
@bors r=oli-obk |
…iaskrgr Rollup of 3 pull requests Successful merges: - rust-lang#134898 (Make it easier to run CI jobs locally) - rust-lang#135195 (Make `lit_to_mir_constant` and `lit_to_const` infallible) - rust-lang#135261 (Account for identity substituted items in symbol mangling) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 3 pull requests Successful merges: - rust-lang#134898 (Make it easier to run CI jobs locally) - rust-lang#135195 (Make `lit_to_mir_constant` and `lit_to_const` infallible) - rust-lang#135261 (Account for identity substituted items in symbol mangling) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#135261 - compiler-errors:coverage-has-identity-substs, r=oli-obk Account for identity substituted items in symbol mangling See the inline comment. r? oli-obk Fixes rust-lang#135235
See the inline comment.
r? oli-obk
Fixes #135235