Skip to content

wf-check types of ty-level consts before evaluating them in KnownPanicslint - #164294

Open
RuleOfSix wants to merge 1 commit into
rust-lang:mainfrom
RuleOfSix:ice-unsized-enum-field-mgca
Open

RuleOfSix wants to merge 1 commit into
rust-lang:mainfrom
RuleOfSix:ice-unsized-enum-field-mgca

Conversation

@RuleOfSix

@RuleOfSix RuleOfSix commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #163901.

Associated const mgca items whose type is an enum with an unsized field fail wf-check, but this lint still tries to run, eventually trying to evaluate the const and causing an ICE.

@rustbot

rustbot commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

HIR ty lowering was modified

cc @fmease

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 14, 2026
@rustbot

rustbot commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

r? @khyperia

rustbot has assigned @khyperia.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 77 candidates
  • Random selection from 18 candidates

@khyperia

Copy link
Copy Markdown
Member

thanks for the PR!

hmm, this seems like the wrong place to be checking this - we can't do trait solving during ty lowering.

I think ideally, the error from the not-wf type would propagate somehow and we don't have to reimplement a check like this, but, yeah, not sure if that's possible or how to do that.

@khyperia khyperia added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2026
@RuleOfSix
RuleOfSix force-pushed the ice-unsized-enum-field-mgca branch from b413e7f to 31731ae Compare October 17, 2026 09:16
@rustbot

rustbot commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@RuleOfSix

This comment was marked as outdated.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 17, 2026
@RuleOfSix RuleOfSix changed the title Check for enums with unsized variants when lowering const args to ty::Const Reject tainted bodies in KnownPanicsLint.run_lint Oct 17, 2026
@RuleOfSix RuleOfSix changed the title Reject tainted bodies in KnownPanicsLint.run_lint Ignore tainted bodies in KnownPanicsLint.run_lint Oct 17, 2026
@RuleOfSix
RuleOfSix force-pushed the ice-unsized-enum-field-mgca branch from 31731ae to 6339655 Compare October 18, 2026 22:30
@RuleOfSix RuleOfSix changed the title Ignore tainted bodies in KnownPanicsLint.run_lint wf-check types of ty-level consts before evaluating them in KnownPanicslint Oct 18, 2026
@RuleOfSix

Copy link
Copy Markdown
Contributor Author

I've implemented a tentative fix directly in KnownPanicsLint that very simply checks the type of ty-level constants for well-formedness before attempting to evaluate them. This lint is apparently the only place where this issue occurs, at least in the cases I've checked.

I also fixed a problem with the test: the compiler would only ICE if it generated MIR, so the test was previously always succeeding. Adding --emit=mir to the compile flags for the test fixed that.

Still unsure if this is the best way to solve this problem, but it does catch all of the simple cases. Type-level constants with ill-formed types inside a Const::Unevaluated still cause crashes when this lint runs.

@khyperia

khyperia commented Nov 4, 2026

Copy link
Copy Markdown
Member

sorry for the long/slow response! #165070 has finally merged, and with it, it seems like the test no longer repros. long story short:

  • in your test in this PR, fn main() { Foo::ASSOC } now lowers to a mir::Const::Unevaluated(Foo::ASSOC) rather than a mir::Const::Ty(Foo::ASSOC).
  • this refers to a tiny synthetic MIR thunk whose body is just return mir::Const::Ty(Foo::ASSOC)
  • which then actually does the const fetching thing

I expected the PR to fix issues like this, but to be honest I'm not sure of the exact mechanism in which this is no longer ICE'ing, it may be the case that we can still ICE somehow with a different repro.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ICE Result::unwrap() on an Err value: Unknown(E)

3 participants