-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Move a const-prop-lint specific hack from mir interpret to const-prop-lint and make it fallible #109938
Conversation
…-lint and make it fallible
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @WaffleLapkin (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
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.
ty, r=me
@bors r+ |
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#109723 (Pull some tuple variant fields out into their own struct) - rust-lang#109838 (Fix `non_exhaustive_omitted_patterns` lint span) - rust-lang#109901 (Enforce VarDebugInfo::Place in MIR validation.) - rust-lang#109913 (Doc-comment `IndexVec::from_elem` and use it in a few more places) - rust-lang#109914 (Emit feature error for parenthesized generics in associated type bounds) - rust-lang#109919 (rustdoc: escape GAT args in more cases) - rust-lang#109937 (Don't collect return-position impl traits for documentation) - rust-lang#109938 (Move a const-prop-lint specific hack from mir interpret to const-prop-lint and make it fallible) - rust-lang#109940 (Add regression test for rust-lang#93911) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
// that the `RevealAll` pass has happened and that the body's consts | ||
// are normalized, so any call to resolve before that needs to be | ||
// manually normalized. | ||
let val = self.tcx.try_normalize_erasing_regions(self.param_env, c.literal).ok()?; |
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.
Does this needs to be added to the equivalent method in the ConstProp
opt too?
Strange, why did the bot not ping wg-const-eval here? Though you also got the first-time contributor message, so clearly the bot was confused... |
Oli is special and (always?) usually gets the first-time contributor message. |
Oh, fascinating. Seems worth a bug report for the bot though? Is there an issue tracking this? |
I'm not aware of one but also don't know where one would be reported. |
…troalbini [stable] Prepare Rust 1.69.0 Last minute backports: * rust-lang#109643 * rust-lang#110135 * rust-lang#109938 * rust-lang#109937 * rust-lang#109266 This PR also bumps the channel to stable, and backports the release notes from master. r? `@ghost` cc `@rust-lang/release`
fixes #109743
This hack didn't need to live in the mir interpreter. For const-prop-lint it is entirely correct to avoid doing any const prop if normalization fails at this stage. Most likely we couldn't const propagate anything anyway, and if revealing was needed (so opaque types were involved), we wouldn't want to be too smart and leak the hidden type anyway.