-
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
handle ConstKind::Unresolved after monomorphizing #70249
Conversation
// repeatedly monomorphize generic parameters until we either | ||
// reach a value or an error. |
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.
This is in the wrong direction. You should never monomorphize more than once.
If the result isn't monomorphic, something went very wrong.
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.
@davidtwco and I recently spent some time tracking down cases of multiple monomorphization, as they broke down their "polymorphization" work (right now nothing happens if you apply the same substitutions more than once in codegen, because the substitutions themselves are monomorphic, but that doesn't make it correct, and it certainly breaks down with polymorphization).
} | ||
ty::ConstKind::Value(value) => Ok(value), | ||
_ => { | ||
let const_ = self.monomorphize(&constant.literal); |
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.
What you could try is first always monomorphize
-ing, and then checking for Unevaluated
(you'd need to remove let substs = self.monomorphize(&substs);
).
It's not the right fix here (see #70125 (comment)), but it could be useful for handling the erroring constant case.
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.
ok, this already seems to fix all tests 🤔 Will still look into your comment on #70125
match self.monomorphize(&constant.literal).val { | ||
ty::ConstKind::Unevaluated(def_id, substs, promoted) => self | ||
.cx | ||
.tcx() | ||
.const_eval_resolve(ty::ParamEnv::reveal_all(), def_id, substs, promoted, None) | ||
.map_err(|err| { | ||
if promoted.is_none() { | ||
self.cx | ||
.tcx() | ||
.sess | ||
.span_err(constant.span, "erroneous constant encountered"); | ||
} | ||
err | ||
}), |
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.
@oli-obk @wesleywiser @varkor @yodaldevoid NB: this Unevaluated
handling should, after #70125 (comment) is fixed, only handle the error case, whereas success should result in monomorphize
itself doing the evaluation.
What do you think about merging this PR before that root cause is fixed?
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 happy to merge this interim fix if it addresses one of the existing ICEs 👍
@bors r+ as per #70249 (comment) |
📌 Commit 8533778 has been approved by |
handle ConstKind::Unresolved after monomorphizing fixes rust-lang#70125 r? @bjorn3
handle ConstKind::Unresolved after monomorphizing fixes rust-lang#70125 r? @bjorn3
Rollup of 9 pull requests Successful merges: - rust-lang#69251 (#[track_caller] in traits) - rust-lang#69880 (miri engine: turn error sanity checks into assertions) - rust-lang#70207 (Use getentropy(2) on macos) - rust-lang#70227 (Only display definition when suggesting a typo) - rust-lang#70236 (resolve: Avoid "self-confirming" import resolutions in one more case) - rust-lang#70248 (parser: simplify & remove unused field) - rust-lang#70249 (handle ConstKind::Unresolved after monomorphizing) - rust-lang#70269 (remove redundant closures (clippy::redundant_closure)) - rust-lang#70270 (Clean up E0449 explanation) Failed merges: r? @ghost
fixes #70125
r? @bjorn3