Skip to content

Cleanup from_cycle_error.rs#153709

Open
zetanumbers wants to merge 3 commits intorust-lang:mainfrom
zetanumbers:from_cycle_error_cleanup
Open

Cleanup from_cycle_error.rs#153709
zetanumbers wants to merge 3 commits intorust-lang:mainfrom
zetanumbers:from_cycle_error_cleanup

Conversation

@zetanumbers
Copy link
Contributor

@zetanumbers zetanumbers commented Mar 11, 2026 •
edited
Loading

A cleanup based and blocked 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
Copy link
Collaborator

rustbot commented Mar 11, 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

nnethercote commented Mar 11, 2026

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

zetanumbers commented Mar 11, 2026

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

nnethercote commented Mar 11, 2026

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
Copy link
Collaborator

rust-log-analyzer commented Mar 11, 2026

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

Click to see the possible cause of the failure (guessed by this bot) 
##[endgroup]
Executing "/scripts/stage_2_test_set1.sh"
+ /scripts/stage_2_test_set1.sh
+ '[' 1 == 1 ']'
+ echo 'PR_CI_JOB set; skipping tidy'
+ SKIP_TIDY='--skip tidy'
+ ../x.py --stage 2 test --skip tidy --skip compiler --skip src
PR_CI_JOB set; skipping tidy
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.05s
##[endgroup]
---

4 LL |     fn g() -> B;
5    |               ^
6    |
- help: `B` is dyn-incompatible, use `impl B` to return an opaque type, as long as you return a single underlying type
8    |
9 LL |     fn g() -> impl B;
10    |               ++++


The actual stderr differed from the expected stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args parallel-rustc/fn-sig-cycle-ice-153391.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/parallel-rustc/fn-sig-cycle-ice-153391.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/parallel-rustc/fn-sig-cycle-ice-153391" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers" "--edition=2024" "-Z" "threads=16"
stdout: none
--- stderr -------------------------------
error[E0782]: expected a type, found a trait
##[error]  --> /checkout/tests/ui/parallel-rustc/fn-sig-cycle-ice-153391.rs:13:23
   |
LL |     fn bar(&self, x: &A);
   |                       ^
   |
   = note: `A` is dyn-incompatible, otherwise a trait object could be used
help: use a new generic type parameter, constrained by `A`
   |
LL -     fn bar(&self, x: &A);
LL +     fn bar<T: A>(&self, x: &T);
   |
help: you can also use an opaque type, but users won't be able to specify the type parameter when calling the `fn`, having to rely exclusively on type inference
   |
LL |     fn bar(&self, x: &impl A);
   |                       ++++
---
LL |     fn g() -> impl B;
   |               ++++
help: alternatively, you can return an owned trait object
   |
LL |     fn g() -> Box<dyn B>;
   |               +++++++  +

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0782`.
------------------------------------------

@zetanumbers
Copy link
Contributor Author

zetanumbers commented Mar 11, 2026

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!.

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

Reviewers

No reviews

Assignees

@nnethercote nnethercote

Labels

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ICE: only 'variances_of' returns '&[ty::Variance]'

5 participants

@zetanumbers @rustbot @nnethercote @rust-log-analyzer @TKanX