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.
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.
@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.
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.
ok, this already seems to fix all tests 🤔 Will still look into your comment on #70125
lcnr
changed the title
| 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.
@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.
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 |
bors
added
the
S-waiting-on-bors
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