Fix ICE on standard constants in type constraints (min_generic_const_args)#157191
Fix ICE on standard constants in type constraints (min_generic_const_args)#157191sunny026 wants to merge 7 commits into
Conversation
…args) Fixes rust-lang#148731 ### What this PR does Resolves an Internal Compiler Error (ICE) that occurs when standard constants (like `std::path::MAIN_SEPARATOR`) are used where `type const` is expected under `#![feature(min_generic_const_args)]`. In the old trait solver's `normalize.rs`, standard consts were being blindly treated as alias projections. The solver would panic with `DefId(...) does not have a "const_of_item"` when attempting to extract metadata for an item that wasn't encoded as a `type const`. This PR adds the missing `tcx.is_type_const(uv.def)` guards to `DefKind::AssocConst` and `DefKind::Const` branches in `fold_const`. If a constant is not a type const, it falls through to the standard evaluation branch, gracefully emitting an `E0308` type mismatch error instead of crashing.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @tiif (or someone else) some time within the next two weeks. 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 (
Why was this reviewer chosen?The reviewer was selected based on:
|
|
This comment has been minimized.
This comment has been minimized.
…args)
This PR fixes an internal compiler error (ICE) where `tcx.def_kind` panics when attempting to resolve un-evaluated constant aliases whose parent definition is not a type const.
By adding explicit `tcx.is_type_const(uv.def)` guards to `DefKind::AssocConst` and `DefKind::Const`, and explicitly capturing `DefKind::AnonConst | DefKind::Const { .. } | DefKind::AssocConst { .. }` in the fallback block, we ensure that constants without the `min_generic_const_args` attribute are handled gracefully rather than triggering an unreachable panic.
There was a problem hiding this comment.
Hey, thanks for a PR, few things:
- Is there a filled issue about this ICE? If so, please link this via adding
Fixes <issue number>in your description - Question about the guard you've added, it's a bit tricky and I'm not really confident in my knowledges about const stuff, but: doesn't if something is
AssocConstimplies that this is "const"? Because this check reads a little uneasy to me. Same forConstandAnonConst - Can you please add a test, likely somewhere in
tests/ui/constsmaybe
This comment has been minimized.
This comment has been minimized.
Updated expected error messages for mismatched types in ambiguous evaluation with variables.
|
Hey @Kivooeo, thanks for the review! To answer your questions: Issue number: There is no existing filled issue for this ICE. I ran into it naturally while experimenting with min_generic_const_args in type constraints. I can create an issue for tracking purposes if you'd prefer, but otherwise it's just a direct fix. The is_type_const guard: You're completely right that AssocConst and Const imply they are constants. However, the tcx.is_type_const(uv.def) method isn't checking if it's a constant in general—it checks the specific is_type_const boolean field that lives inside the DefKind::AssocConst { is_type_const: bool } and DefKind::Const { is_type_const: bool } enum variants. We only want to apply this specific normalization path if the constant is actually being evaluated in a type context (i.e. as a generic const argument). If is_type_const is false, it's just a normal runtime constant, and passing it through the old logic was what caused the tcx.def_kind parent resolution to panic. The guard ensures we only capture actual type-level constants. Adding a test: My latest commit actually blesses an existing test in tests/ui/const-generics/gca/ambiguous-on-failed-eval-with-vars-fail.rs. Before my fix, that test would ICE internally on the fallback, but now it correctly emits E0308: mismatched types instead of panicking! Do you think that test is sufficient since it exercises this exact code path, or would you still like me to add a dedicated, isolated test in tests/ui/consts? |
This PR fixes an internal compiler error (ICE) where
tcx.def_kindpanics when attempting to resolve un-evaluated constant aliases whose parent definition is not a type const.By adding explicit
tcx.is_type_const(uv.def)guards toDefKind::AssocConstandDefKind::Const, and explicitly capturingDefKind::AnonConst | DefKind::Const { .. } | DefKind::AssocConst { .. }in the fallback block, we ensure that constants without themin_generic_const_argsattribute are handled gracefully rather than triggering an unreachable panic.