-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
use ObligationCtxt
not QueryNormalizer
in rustdoc's normalization
#108503
use ObligationCtxt
not QueryNormalizer
in rustdoc's normalization
#108503
Conversation
r? @notriddle (rustbot has picked a reviewer for you, use r? to override) |
r? types |
Let's re-roll. r? types |
r=me after rebase @rustbot author |
@oli-obk do you know what's going on here? or who we should ask? |
a2868c5
to
390246c
Compare
@bors r=oli-obk |
⌛ Testing commit d0c308c with merge a14c185f4330588cd23391993cb86940913a391d... |
💔 Test failed - checks-actions |
@bors retry LLVM build failed |
☔ The latest upstream changes (presumably #114264) made this pull request unmergeable. Please resolve the merge conflicts. |
Locally I've rebased the commits and run @BoxyUwU Would you be willing to bring this PR up to speed and re |
Fyi, your PR seems to fix #112242. If you could add a regression test for it, that would be awesome! :) |
d0c308c
to
2ccfd04
Compare
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.
Thanks a lot!
@bors r=oli-obk,fmease |
…izer, r=oli-obk,fmease use `ObligationCtxt` not `QueryNormalizer` in rustdoc's `normalization` `QueryNormalizer` doesn't handle not-well-formed projections or ambiguity so should not be used by rustdoc as rustdoc happens on code that is not well formed. This PR replaces the usage of `QueryNormalizer` with `ObligationCtxt::normalize` which is designed to work on not-wf code while not being in typeck. This also removes two uses of `actually_rustdoc` from the compiler which seems good to me. I am somewhat confused as to the "point" of `QueryNormalizer`, it intends to be "the main way of normalizing" in the future and yet ICEs when encountering not wf types or when normalization is ambiguous which seems very incompatible with its stated goal since that makes it only suitable for using after typeck?
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
Ah, now I can see that it happens with stage2→stage3, not with stage1→stage2. Let me investigate this ICE… |
@bors r- synchronizing the queue |
☔ The latest upstream changes (presumably #117193) made this pull request unmergeable. Please resolve the merge conflicts. |
@BoxyUwU any updates on the CI failure? |
QueryNormalizer
doesn't handle not-well-formed projections or ambiguity so should not be used by rustdoc as rustdoc happens on code that is not well formed. This PR replaces the usage ofQueryNormalizer
withObligationCtxt::normalize
which is designed to work on not-wf code while not being in typeck. This also removes two uses ofactually_rustdoc
from the compiler which seems good to me.I am somewhat confused as to the "point" of
QueryNormalizer
, it intends to be "the main way of normalizing" in the future and yet ICEs when encountering not wf types or when normalization is ambiguous which seems very incompatible with its stated goal since that makes it only suitable for using after typeck?