Skip to content

Fix hypothetical ICE in variances_of#153709

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
zetanumbers:from_cycle_error_cleanup
Mar 17, 2026
Merged

Fix hypothetical ICE in variances_of#153709
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
zetanumbers:from_cycle_error_cleanup

Conversation

@zetanumbers
Copy link
Contributor

@zetanumbers zetanumbers commented Mar 11, 2026

A cleanup based on #153694.

This PR changes panic messages in fn_sig's value_from_cycle_error to more fitting ones. Also this PR makes variances_of's value_from_cycle_error to utilize new query key argument instead of extracting it from CycleError.

Closes #127971

r? @nnethercote

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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 Mar 11, 2026
@rustbot

This comment has been minimized.

@nnethercote
Copy link
Contributor

Seems plausible, but please add a short description that explains the current situation and how/why this PR improves things. Currently the only descriptive word is "cleanup" which tells the reader very little.

cc @Zoxc

@zetanumbers
Copy link
Contributor Author

Seems plausible, but please add a short description that explains the current situation and how/why this PR improves things. Currently the only descriptive word is "cleanup" which tells the reader very little.

Updated the top comment. Is it good?

@nnethercote
Copy link
Contributor

It's an improvement, but it still doesn't explain what was going wrong in #127971 and how that has been fixed. The new fn_sig calls default, is that important?

@rust-log-analyzer

This comment has been minimized.

@zetanumbers
Copy link
Contributor Author

It's an improvement, but it still doesn't explain what was going wrong in #127971 and how that has been fixed.

This PR basically eliminates panic that have caused #127971 and avoids fn_sig's issue like #153391 to happen to variances_of.

The new fn_sig calls default, is that important?

Previously there was:

tcx.dcx().abort_if_errors();
unreachable!()

Now compare it to default's definition:

let Some(guar) = tcx.sess.dcx().has_errors() else {
    bug!(
        "`from_cycle_error_default` on query `{query_name}` called without errors: {:#?}",
        cycle_error.cycle,
    );
};
guar.raise_fatal()

These are essentially the same. I've only changed unreachable! to more descriptive bug!.

@rust-bors

This comment has been minimized.

@nnethercote
Copy link
Contributor

A better PR title would be something like "Fix ICE in variances_of." That makes it clear that it's not just a cleanup, but also fixing a bug.

A better commit message would be something like this:

variances_of currently used search_for_cycle_permutation, which can fail and abort when <... describe circumstances... >. This commit changes variances_of to receive a def_id which means it can compute a value without using search_for_cycle_permutation, avoiding the possible abort.

Fixes #127971

This explains the current situation and the change in a way that is meaningful to a reader who isn't already familiar with #127971.

@nnethercote
Copy link
Contributor

Needs rebasing over #153694, and the title/description/commit messages updated as described above.

@nnethercote nnethercote 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 Mar 13, 2026
@zetanumbers zetanumbers changed the title Cleanup from_cycle_error.rs Fix hypothetical ICE in variances_of Mar 16, 2026
@zetanumbers zetanumbers force-pushed the from_cycle_error_cleanup branch from b597c04 to 567f8bd Compare March 16, 2026 09:32
@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@zetanumbers
Copy link
Contributor Author

@rustbot ready

@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 Mar 16, 2026
@nnethercote
Copy link
Contributor

@bors r+ rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 17, 2026

📌 Commit 567f8bd has been approved by nnethercote

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 17, 2026
Zalathar added a commit to Zalathar/rust that referenced this pull request Mar 17, 2026
…up, r=nnethercote

Fix hypothetical ICE in `variances_of`

A cleanup based on rust-lang#153694.

~~This PR changes panic messages in `fn_sig`'s `value_from_cycle_error` to more fitting ones.~~ Also this PR makes `variances_of`'s `value_from_cycle_error` to utilize new query key argument instead of extracting it from `CycleError`.

Closes rust-lang#127971

r? @nnethercote
@rust-bors rust-bors bot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 17, 2026
@rust-bors rust-bors bot removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 17, 2026
@rust-bors

This comment has been minimized.

variances_of currently used search_for_cycle_permutation, which can fail and abort when constructed error result value does not match query input.
This commit changes variances_of to receive a def_id which means it can compute a value without using search_for_cycle_permutation, avoiding the possible abort.

Fixes rust-lang#127971
@zetanumbers zetanumbers force-pushed the from_cycle_error_cleanup branch from 567f8bd to b5e53dc Compare March 17, 2026 10:52
@rustbot
Copy link
Collaborator

rustbot commented Mar 17, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rustbot
Copy link
Collaborator

rustbot commented Mar 17, 2026

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

@nnethercote
Copy link
Contributor

@bors r+

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 17, 2026

📌 Commit b5e53dc has been approved by nnethercote

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 17, 2026
Zalathar added a commit to Zalathar/rust that referenced this pull request Mar 17, 2026
…up, r=nnethercote

Fix hypothetical ICE in `variances_of`

A cleanup based on rust-lang#153694.

~~This PR changes panic messages in `fn_sig`'s `value_from_cycle_error` to more fitting ones.~~ Also this PR makes `variances_of`'s `value_from_cycle_error` to utilize new query key argument instead of extracting it from `CycleError`.

Closes rust-lang#127971

r? @nnethercote
rust-bors bot pushed a commit that referenced this pull request Mar 17, 2026
Rollup of 7 pull requests

Successful merges:

 - #153801 (Add the option to run UI tests with the parallel frontend)
 - #153967 (Tweak wording of failed predicate in inference error)
 - #152968 (Flip "region lattice" in RegionKind doc comment)
 - #153531 (Fix LegacyKeyValueFormat report from docker build: various)
 - #153709 (Fix hypothetical ICE in `variances_of`)
 - #153884 (test `classify-runtime-const` for `f16`)
 - #153946 (dissolve `tests/ui/cross`)
rust-bors bot pushed a commit that referenced this pull request Mar 17, 2026
…uwer

Rollup of 14 pull requests

Successful merges:

 - #153972 (stdarch subtree update)
 - #153801 (Add the option to run UI tests with the parallel frontend)
 - #153959 (Fix non-module `parent_module` in stripped cfg diagnostics)
 - #153967 (Tweak wording of failed predicate in inference error)
 - #152968 (Flip "region lattice" in RegionKind doc comment)
 - #153531 (Fix LegacyKeyValueFormat report from docker build: various)
 - #153622 (remove concept of soft-unstable features)
 - #153709 (Fix hypothetical ICE in `variances_of`)
 - #153884 (test `classify-runtime-const` for `f16`)
 - #153894 (Point at unit structs on foreign crates in type errors when they are the pattern of a binding)
 - #153920 (improve `#[track_caller]` invalid ABI error)
 - #153946 (dissolve `tests/ui/cross`)
 - #153965 (Fix minor kasan bugs)
 - #153991 (Small report_cycle refactor)
@rust-bors rust-bors bot merged commit d8360a9 into rust-lang:main Mar 17, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 17, 2026
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Mar 18, 2026
…uwer

Rollup of 14 pull requests

Successful merges:

 - rust-lang/rust#153972 (stdarch subtree update)
 - rust-lang/rust#153801 (Add the option to run UI tests with the parallel frontend)
 - rust-lang/rust#153959 (Fix non-module `parent_module` in stripped cfg diagnostics)
 - rust-lang/rust#153967 (Tweak wording of failed predicate in inference error)
 - rust-lang/rust#152968 (Flip "region lattice" in RegionKind doc comment)
 - rust-lang/rust#153531 (Fix LegacyKeyValueFormat report from docker build: various)
 - rust-lang/rust#153622 (remove concept of soft-unstable features)
 - rust-lang/rust#153709 (Fix hypothetical ICE in `variances_of`)
 - rust-lang/rust#153884 (test `classify-runtime-const` for `f16`)
 - rust-lang/rust#153894 (Point at unit structs on foreign crates in type errors when they are the pattern of a binding)
 - rust-lang/rust#153920 (improve `#[track_caller]` invalid ABI error)
 - rust-lang/rust#153946 (dissolve `tests/ui/cross`)
 - rust-lang/rust#153965 (Fix minor kasan bugs)
 - rust-lang/rust#153991 (Small report_cycle refactor)
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Mar 18, 2026
…uwer

Rollup of 14 pull requests

Successful merges:

 - rust-lang/rust#153972 (stdarch subtree update)
 - rust-lang/rust#153801 (Add the option to run UI tests with the parallel frontend)
 - rust-lang/rust#153959 (Fix non-module `parent_module` in stripped cfg diagnostics)
 - rust-lang/rust#153967 (Tweak wording of failed predicate in inference error)
 - rust-lang/rust#152968 (Flip "region lattice" in RegionKind doc comment)
 - rust-lang/rust#153531 (Fix LegacyKeyValueFormat report from docker build: various)
 - rust-lang/rust#153622 (remove concept of soft-unstable features)
 - rust-lang/rust#153709 (Fix hypothetical ICE in `variances_of`)
 - rust-lang/rust#153884 (test `classify-runtime-const` for `f16`)
 - rust-lang/rust#153894 (Point at unit structs on foreign crates in type errors when they are the pattern of a binding)
 - rust-lang/rust#153920 (improve `#[track_caller]` invalid ABI error)
 - rust-lang/rust#153946 (dissolve `tests/ui/cross`)
 - rust-lang/rust#153965 (Fix minor kasan bugs)
 - rust-lang/rust#153991 (Small report_cycle refactor)
@zetanumbers zetanumbers deleted the from_cycle_error_cleanup branch March 18, 2026 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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: only 'variances_of' returns '&[ty::Variance]'

4 participants