-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Stop passing resolver disambiguator state to AST lowering. #143361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
DefPathData::Ctor => 'c', | ||
DefPathData::AnonConst => 'k', | ||
DefPathData::AnonConst => 'K', | ||
DefPathData::LateAnonConst => 'k', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not certain changing mangling is the best idea. Is there some update to a demangler to be made before this PR is accepted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised you're not getting the demangler roundtrip ICE, but most likely this code path isn't hit in codegen tests with v0 mangling.
I don't think it's possible without changing mangling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find where in the demangler code this is used. IIUC, there are only two categories that are checked, 'C' and 'S', the latter not being produced any more. Should I look somewhere else?
@rustbot author Please add a codegen test (which will probably ICE if the compiler is built with debug assertions as the demangler can't support this new syntax yet). |
Reminder, once the PR becomes ready for a review, use |
☔ The latest upstream changes (presumably #144249) made this pull request unmergeable. Please resolve the merge conflicts. |
4b55098
to
235f4df
Compare
Some changes occurred in tests/codegen-llvm/sanitizer cc @rcvalle |
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
AST->HIR lowering can use a disjoint set of
DefPathData
as the resolver, so we don't need to pass the disambiguator state.r? @oli-obk