Improved handling for cycles within the query system itself

[jyn514]

This PR is somewhat complicated and I don't expect it to merge as-is. Let me start with a motivating example. The following rustc-driver worked fine a week ago:

struct Driver;

impl Callbacks for Driver {
    fn config(&mut self, config: &mut interface::Config) {
        config.override_queries = Some(|_sess, providers| {
            providers.queries.codegen_fn_attrs = |tcx, def_id| {
                let _ = tcx.hir_crate_items(());
                (rustc_interface::DEFAULT_QUERY_PROVIDERS.queries.codegen_fn_attrs)(tcx, def_id)
            };
        });

    }
}

fn main() -> ExitCode {
    rustc_driver::install_ice_hook("https://github.com/ferrocene/ferrocene/issues/new", |_| ());
    let handler = EarlyDiagCtxt::new(ErrorOutputType::default());
    rustc_driver::init_rustc_env_logger(&handler);
    rustc_driver::catch_with_exit_code(move || {
        let args: Vec<String> = std::env::args().collect();
        rustc_driver::run_compiler(&args, &mut Driver)
    })
}

Since #153489 merged, it hangs indefinitely trying to compile the following program (reduced from tests/ui/delegation/bad-resolve):

#![feature(fn_delegation)]
#![allow(incomplete_features)]

trait Trait {
    type Type;
    fn bar() {}
}

struct F;
impl Trait for F {
    type Type = i32;
}

struct S(F);
impl Trait for S {
    reuse <F as Trait>::bar;
}

fn main() {}

I'll step through how this program behaves on each commit in this PR.

---

1. Main branch: Hangs indefinitely with no output.

2. After first commit (adding safeguards to ensure_sufficient_stack):
   a. First, it prints a stacktrace that cuts off early
   b. Then it hangs forever.

3. After calling abort if ensure_sufficient_stack fails:
   a. First, it prints a ~250 frame stacktrace,
   including both the first and last hundred frames.
   b. Then it exits.

4. After adding debug logging, shows the stack trace from above, but first:
   a. Prints a trace of the query execution
   b. Shows the stacktrace from above.

5. After the bug fix: as above, but instead of the 5 million frame backtrace, prints a "more normal" query cycle:

error[E0391]: cycle detected when looking up span for `<impl at bad-resolve.rs:15:1: 15:17>::bar`
  |
  = note: ...which requires getting HIR ID of `<impl at bad-resolve.rs:15:1: 15:17>::bar`...
  = note: ...which again requires looking up span for `<impl at bad-resolve.rs:15:1: 15:17>::bar`, completing the cycle
note: cycle used when computing codegen attributes of `Trait::bar`
 --> bad-resolve.rs:6:5
  |
6 |     fn bar() {}
  |     ^^^^^^^^

---

Here's an extract of that query trace:

Details

rustc_query_impl::execution::try_execute_query query="hir_crate_items"
  ...
  rustc_query_impl::execution::try_execute_query query="lower_delayed_owner"
    ...
    rustc_query_impl::execution::try_execute_query query="codegen_fn_attrs"
      rustc_query_impl::execution::try_execute_query query="hir_crate_items"
        0ms INFO rustc_query_impl::execution hit a query cycle evaluating hir_crate_items!
          rustc_query_impl::execution::try_execute_query query="def_span"
            rustc_query_impl::execution::try_execute_query query="local_def_id_to_hir_id"
              rustc_query_impl::execution::try_execute_query query="lower_delayed_owner"
                0ms INFO rustc_query_impl::execution hit a query cycle evaluating lower_delayed_owner!
                  rustc_query_impl::execution::try_execute_query query="def_span"
                    0ms INFO rustc_query_impl::execution hit a query cycle evaluating def_span!
                    rustc_query_impl::execution::try_execute_query query="def_span"
...

I feel somewhat strongly that there is a bug here; it should not be possible to get query cycles within def_span.
In particular, note that the first two cycles are ignored altogether in order to report the
third cycle; that seems quite bad!

I'm not sure if the bug is in the implementation of lower_delayed_owner, in the query system, or merely in the fact that lower_delay_owner is a prerequisite of hir_items; I'm opening this PR to get more feedback on that. However, I think the commits here stand on their own.

In #153979 (comment) I mistakenly attributed this to recent refactors in the query system; I no longer think that's true, I think the infinite loop is a latent bug that's been around for years, and the new query cycle is related to lower_delayed_owner, not the query system itself.

cc @petrochenkov @aerooneqq @Zalathar

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

[rustbot]

r? @JohnTitor

rustbot has assigned @JohnTitor.
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 69 candidates
• Random selection from 12 candidates

[jyn514]

hm. maybe r? @Zalathar ?

[rustbot]

Zalathar is not on the review rotation at the moment.
They may take a while to respond.

[rust-log-analyzer]

The job aarch64-gnu-llvm-21-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.04s
##[endgroup]
---
To only update this specific test, also pass `--test-args query-system/query-cycle-printing-issue-151358.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/query-system/query-cycle-printing-issue-151358.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/query-system/query-cycle-printing-issue-151358" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "incomplete_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"
stdout: none
--- stderr -------------------------------
error[E0391]: cycle detected when getting the resolver for lowering
   |
   = note: ...which requires getting HIR ID of `Default`...
---
To only update this specific test, also pass `--test-args resolve/query-cycle-issue-124901.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/resolve/query-cycle-issue-124901.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/resolve/query-cycle-issue-124901" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "incomplete_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"
stdout: none
--- stderr -------------------------------
error[E0391]: cycle detected when getting the resolver for lowering
   |
   = note: ...which requires getting HIR ID of `Default`...

[Zoxc]

Not sure about the query related changes here, but it makes sense to have proper stack limit so we don't OOM with infinite recursion.

[bjorn3]

This can just be Cell<u16>, right?

[bjorn3]

I'm pretty sure this new implementation breaks on targets where stacker uses the default impl of not growing the stack. There remaining_stack would return None and thus on ever recursion TIMES_GROWN would be increased, only allowing a 1000x recursion rather than 1000 * however many recursions a single stack segment fits.

[Zoxc]

@jyn514 You could try your test failures with #154389. We have some failing tests on main which that detects and the cause might be similar.