-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Fix ICE: Don't try to evaluate type_consts when eagerly collecting items #151441
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
Conversation
| @@ -0,0 +1,11 @@ | |||
| //@ check-pass | |||
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.
can you add a multi-crate test such as:
// crate a
#[type_const]
pub const FOO: usize = 1;
// crate b
fn main() {
println!("{}", a::FOO);
}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.
though that might ICE until #151260 is merged? unsure what the overlap here is
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 did not really see any overlap, you might want to ask @reddevilmidzy about that, but yea such a test would currently ICE. Might be better to make that part of #151260?
Update compiler/rustc_monomorphize/src/collector.rs Add FIXME(mgca) comment to potentially re-investigate in the future. Co-Authored-By: Boxy <rust@boxyuwu.dev>
|
@BoxyUwU was there anything other than that FIXME comment for future investigation? |
|
@bors r+ rollup |
Fix ICE: Don't try to evaluate type_consts when eagerly collecting items This fixes rust-lang#151246 The change is pretty straightforward if the Monomorphization strategy is eager which `-Clink-dead-code=true` sets. This then would lead to the existing code to try and evaluate a `type const` which does not have a body to evaluate leading to an ICE. The change is pretty straight forward just skip over type consts. This also seems like a sensible choice to me since a MonoItem can only be a Fn, Static, or Asm. A type const is none of the aforementioned. And even if it was added to the MonoItems list it would then later fail this check: https://github.com/rust-lang/rust/blob/fe98ddcfcfb6f185dbf4adeaf439d8a756da0273/compiler/rustc_monomorphize/src/collector.rs#L438-L440 Since that explicitly checks that the MonoItem's `DefKind` is static and not anything else. One more change is the addition of a simple test of the example code from rust-lang#151246 that checks that code compiles successfully with `-Clink-dead-code=true`. The only other change was to make the guard checks a little easier to read by making the logic more linear instead of one big if statement. r? @BoxyUwU @rustbot label +F-associated_const_equality +F-min_generic_const_args
…uwer Rollup of 8 pull requests Successful merges: - #148206 (Deduplicated float tests and unified in floats/mod.rs) - #151042 (fix fallback impl for select_unpredictable intrinsic) - #151220 (option: Use Option::map in Option::cloned) - #151260 (Handle unevaluated ConstKind in in_operand) - #151441 (Fix ICE: Don't try to evaluate type_consts when eagerly collecting items) - #151465 (codegen: clarify some variable names around function calls) - #151469 (llvm: Tolerate dead_on_return attribute changes) - #151476 (Avoid `-> ()` in derived functions.) r? @ghost
…uwer Rollup of 8 pull requests Successful merges: - #148206 (Deduplicated float tests and unified in floats/mod.rs) - #151042 (fix fallback impl for select_unpredictable intrinsic) - #151220 (option: Use Option::map in Option::cloned) - #151260 (Handle unevaluated ConstKind in in_operand) - #151441 (Fix ICE: Don't try to evaluate type_consts when eagerly collecting items) - #151465 (codegen: clarify some variable names around function calls) - #151469 (llvm: Tolerate dead_on_return attribute changes) - #151476 (Avoid `-> ()` in derived functions.) r? @ghost
…uwer Rollup of 8 pull requests Successful merges: - #148206 (Deduplicated float tests and unified in floats/mod.rs) - #151042 (fix fallback impl for select_unpredictable intrinsic) - #151220 (option: Use Option::map in Option::cloned) - #151260 (Handle unevaluated ConstKind in in_operand) - #151441 (Fix ICE: Don't try to evaluate type_consts when eagerly collecting items) - #151465 (codegen: clarify some variable names around function calls) - #151469 (llvm: Tolerate dead_on_return attribute changes) - #151476 (Avoid `-> ()` in derived functions.) r? @ghost
…uwer Rollup of 11 pull requests Successful merges: - #151001 (rustdoc: render doc(hidden) as a code attribute) - #151042 (fix fallback impl for select_unpredictable intrinsic) - #151220 (option: Use Option::map in Option::cloned) - #151260 (Handle unevaluated ConstKind in in_operand) - #151296 (MGCA: Fix incorrect pretty printing of valtree arrays) - #151423 (Move assert_matches to planned stable path) - #151441 (Fix ICE: Don't try to evaluate type_consts when eagerly collecting items) - #151465 (codegen: clarify some variable names around function calls) - #151468 (fix `f16` doctest FIXMEs) - #151469 (llvm: Tolerate dead_on_return attribute changes) - #151476 (Avoid `-> ()` in derived functions.) r? @ghost
…uwer Rollup of 11 pull requests Successful merges: - #151001 (rustdoc: render doc(hidden) as a code attribute) - #151042 (fix fallback impl for select_unpredictable intrinsic) - #151220 (option: Use Option::map in Option::cloned) - #151260 (Handle unevaluated ConstKind in in_operand) - #151296 (MGCA: Fix incorrect pretty printing of valtree arrays) - #151423 (Move assert_matches to planned stable path) - #151441 (Fix ICE: Don't try to evaluate type_consts when eagerly collecting items) - #151465 (codegen: clarify some variable names around function calls) - #151468 (fix `f16` doctest FIXMEs) - #151469 (llvm: Tolerate dead_on_return attribute changes) - #151476 (Avoid `-> ()` in derived functions.) r? @ghost
…uwer Rollup of 11 pull requests Successful merges: - #151001 (rustdoc: render doc(hidden) as a code attribute) - #151042 (fix fallback impl for select_unpredictable intrinsic) - #151220 (option: Use Option::map in Option::cloned) - #151260 (Handle unevaluated ConstKind in in_operand) - #151296 (MGCA: Fix incorrect pretty printing of valtree arrays) - #151423 (Move assert_matches to planned stable path) - #151441 (Fix ICE: Don't try to evaluate type_consts when eagerly collecting items) - #151465 (codegen: clarify some variable names around function calls) - #151468 (fix `f16` doctest FIXMEs) - #151469 (llvm: Tolerate dead_on_return attribute changes) - #151476 (Avoid `-> ()` in derived functions.) r? @ghost
Rollup merge of #151441 - Keith-Cancel:mgca3, r=BoxyUwU Fix ICE: Don't try to evaluate type_consts when eagerly collecting items This fixes #151246 The change is pretty straightforward if the Monomorphization strategy is eager which `-Clink-dead-code=true` sets. This then would lead to the existing code to try and evaluate a `type const` which does not have a body to evaluate leading to an ICE. The change is pretty straight forward just skip over type consts. This also seems like a sensible choice to me since a MonoItem can only be a Fn, Static, or Asm. A type const is none of the aforementioned. And even if it was added to the MonoItems list it would then later fail this check: https://github.com/rust-lang/rust/blob/fe98ddcfcfb6f185dbf4adeaf439d8a756da0273/compiler/rustc_monomorphize/src/collector.rs#L438-L440 Since that explicitly checks that the MonoItem's `DefKind` is static and not anything else. One more change is the addition of a simple test of the example code from #151246 that checks that code compiles successfully with `-Clink-dead-code=true`. The only other change was to make the guard checks a little easier to read by making the logic more linear instead of one big if statement. r? @BoxyUwU @rustbot label +F-associated_const_equality +F-min_generic_const_args
…uwer Rollup of 11 pull requests Successful merges: - rust-lang/rust#151001 (rustdoc: render doc(hidden) as a code attribute) - rust-lang/rust#151042 (fix fallback impl for select_unpredictable intrinsic) - rust-lang/rust#151220 (option: Use Option::map in Option::cloned) - rust-lang/rust#151260 (Handle unevaluated ConstKind in in_operand) - rust-lang/rust#151296 (MGCA: Fix incorrect pretty printing of valtree arrays) - rust-lang/rust#151423 (Move assert_matches to planned stable path) - rust-lang/rust#151441 (Fix ICE: Don't try to evaluate type_consts when eagerly collecting items) - rust-lang/rust#151465 (codegen: clarify some variable names around function calls) - rust-lang/rust#151468 (fix `f16` doctest FIXMEs) - rust-lang/rust#151469 (llvm: Tolerate dead_on_return attribute changes) - rust-lang/rust#151476 (Avoid `-> ()` in derived functions.) r? @ghost
This fixes #151246
The change is pretty straightforward if the Monomorphization strategy is eager which
-Clink-dead-code=truesets. This then would lead to the existing code to try and evaluate atype constwhich does not have a body to evaluate leading to an ICE. The change is pretty straight forward just skip over type consts.This also seems like a sensible choice to me since a MonoItem can only be a Fn, Static, or Asm. A type const is none of the aforementioned.
And even if it was added to the MonoItems list it would then later fail this check:
rust/compiler/rustc_monomorphize/src/collector.rs
Lines 438 to 440 in fe98ddc
Since that explicitly checks that the MonoItem's
DefKindis static and not anything else.One more change is the addition of a simple test of the example code from #151246 that checks that code compiles successfully with
-Clink-dead-code=true.The only other change was to make the guard checks a little easier to read by making the logic more linear instead of one big if statement.
r? @BoxyUwU
@rustbot label +F-associated_const_equality +F-min_generic_const_args