Skip to content

Do not ICE on generic const expr referencing missing ty param #142810

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jun 20, 2025

Make ParamEnv::find_ty_from_env return an Option<Ty>> so if a const has a type corresponding to an unknown type parameter, we don't ICE when using generic_const_exprs:

#![allow(incomplete_features)]
#![feature(generic_const_exprs, adt_const_params)]
struct X<
    const FN: () = {
        || {
            let _: [(); B]; //~ ERROR cannot find value `B` in this scope
            //~^ ERROR the constant `FN` is not of type `()`
        };
    },
>;
fn main() {}

Fix #142709.

Make `ParamEnv::find_ty_from_env` return an `Option<Ty>>` so if a const has a type corresponding to an unknown type parameter, we don't ICE when using `generic_const_exprs`:

```rust
#![allow(incomplete_features)]
#![feature(generic_const_exprs, adt_const_params)]
struct X<
    const FN: () = {
        || {
            let _: [(); B]; //~ ERROR cannot find value `B` in this scope
            //~^ ERROR the constant `FN` is not of type `()`
        };
    },
>;
fn main() {}
```
@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2025

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
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

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jun 20, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2025

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@compiler-errors
Copy link
Member

I'm pretty certain it's an explicit design choice that find_ty_from_env ICEs when it cannot find a param in-scope, because it's (almost) always a sign of a more underlying bug with param resolution and the way we set up the anonymous const's generics when GCE is enabled.

Given that GCE is on its way out, I'm not totally sure if this fix should be merged. I'll defer to @BoxyUwU of course.

@compiler-errors
Copy link
Member

Also, could you please write up an explanation for why this actually fixes the bug that it does? Given the subtle interaction, it's not necessarily clear why this ICE triggers in the first place, and why we're trying to look up a const param that's not present in the param-env.

Specifically, there seem to be a lot of pieces that are necessary, like the closure, and some that aren't, like that B parameter can be replaced with any non-trivial generic const.

@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu-llvm-19-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test [crashes] tests/crashes/128094.rs ... ok
test [crashes] tests/crashes/129095.rs ... ok
test [crashes] tests/crashes/129109.rs ... ok
test [crashes] tests/crashes/129425.rs ... ok
2025-06-20T22:00:06.741683Z ERROR compiletest::runtest: fatal error, panic: "crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`."
test [crashes] tests/crashes/129209.rs ... FAILED
test [crashes] tests/crashes/129556.rs ... ok
test [crashes] tests/crashes/130346.rs ... ok
test [crashes] tests/crashes/130395.rs ... ok
test [crashes] tests/crashes/130411.rs ... ok
---
test [crashes] tests/crashes/140577.rs ... ok
test [crashes] tests/crashes/140850.rs ... ok
test [crashes] tests/crashes/140729.rs ... ok
test [crashes] tests/crashes/140860.rs ... ok
2025-06-20T22:00:10.741641Z ERROR compiletest::runtest: fatal error, panic: "crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`."
test [crashes] tests/crashes/140891.rs ... FAILED
test [crashes] tests/crashes/34127.rs ... ignored, only executed when the architecture is x86_64
test [crashes] tests/crashes/54888.rs ... ok
test [crashes] tests/crashes/141124.rs ... ok
test [crashes] tests/crashes/87577.rs ... ok
---
failures:

---- [crashes] tests/crashes/129209.rs stdout ----

error: crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`.

thread '[crashes] tests/crashes/129209.rs' panicked at src/tools/compiletest/src/runtest/crashes.rs:17:18:
fatal error
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- [crashes] tests/crashes/140891.rs stdout ----

error: crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`.

thread '[crashes] tests/crashes/140891.rs' panicked at src/tools/compiletest/src/runtest/crashes.rs:17:18:
fatal error


@BoxyUwU
Copy link
Member

BoxyUwU commented Jun 22, 2025

+1, this function ICEs in this case because it is always wrong to be doing trait solving in the "wrong" environment (or atleast, that's the idea :D). The preferred fix would typically be to track down where we decide what ParamEnv to use, then start using one that properly tracks information about const parameters in scope.

Regardless of whether to go with ur approach or the normal approach of picking the right ParamEnv, I would need to understand where the "wrong" ParamEnv is coming from and where these ConstArgHasType goals are originating from. Which rn I can't get just from reading PR description so an explanation for those things would be helpful :3

@BoxyUwU BoxyUwU 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 Jun 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE:None in compiler\rustc_middle\src\ty\sty.rs
5 participants