From 04e05933b479597f6806b06d5d97f09d85a44b8c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 20 Jan 2026 11:56:19 +1100 Subject: [PATCH] Reintroduce `QueryStackFrame` split. I tried removing it in #151203, to replace it with something simpler. But a couple of fuzzing failures have come up and I don't have a clear picture on how to fix them. So I'm reverting the main part of #151203. This commit also adds the two fuzzing tests. Fixes #151226, #151358. --- compiler/rustc_middle/src/query/mod.rs | 2 +- compiler/rustc_middle/src/query/plumbing.rs | 2 +- compiler/rustc_middle/src/ty/print/pretty.rs | 4 +- compiler/rustc_middle/src/values.rs | 4 +- compiler/rustc_query_impl/src/lib.rs | 11 +- compiler/rustc_query_impl/src/plumbing.rs | 96 ++++++++----- .../rustc_query_system/src/query/config.rs | 5 +- compiler/rustc_query_system/src/query/job.rs | 131 ++++++++++-------- compiler/rustc_query_system/src/query/mod.rs | 129 +++++++++++++---- .../rustc_query_system/src/query/plumbing.rs | 61 ++++---- .../query-cycle-printing-issue-151226.rs | 8 ++ .../query-cycle-printing-issue-151226.stderr | 36 +++++ .../query-cycle-printing-issue-151358.rs | 7 + .../query-cycle-printing-issue-151358.stderr | 9 ++ tests/ui/resolve/query-cycle-issue-124901.rs | 2 +- .../resolve/query-cycle-issue-124901.stderr | 9 +- 16 files changed, 355 insertions(+), 161 deletions(-) create mode 100644 tests/ui/query-system/query-cycle-printing-issue-151226.rs create mode 100644 tests/ui/query-system/query-cycle-printing-issue-151226.stderr create mode 100644 tests/ui/query-system/query-cycle-printing-issue-151358.rs create mode 100644 tests/ui/query-system/query-cycle-printing-issue-151358.stderr diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 2f83f3078e89f..9d17c998a8f29 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -88,7 +88,7 @@ use rustc_index::IndexVec; use rustc_lint_defs::LintId; use rustc_macros::rustc_queries; use rustc_query_system::ich::StableHashingContext; -use rustc_query_system::query::{QueryMode, QueryState}; +use rustc_query_system::query::{QueryMode, QueryStackDeferred, QueryState}; use rustc_session::Limits; use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion}; use rustc_session::cstore::{ diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index df333e68add1a..25c9a0a81ab48 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -431,7 +431,7 @@ macro_rules! define_callbacks { #[derive(Default)] pub struct QueryStates<'tcx> { $( - pub $name: QueryState<$($K)*>, + pub $name: QueryState<$($K)*, QueryStackDeferred<'tcx>>, )* } diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 2a65517de4033..947857dde6e12 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -159,9 +159,7 @@ pub macro with_types_for_signature($e:expr) {{ /// Avoids running any queries during prints. pub macro with_no_queries($e:expr) {{ $crate::ty::print::with_reduced_queries!($crate::ty::print::with_forced_impl_filename_line!( - $crate::ty::print::with_no_trimmed_paths!($crate::ty::print::with_no_visible_paths!( - $crate::ty::print::with_forced_impl_filename_line!($e) - )) + $crate::ty::print::with_no_trimmed_paths!($crate::ty::print::with_no_visible_paths!($e)) )) }} diff --git a/compiler/rustc_middle/src/values.rs b/compiler/rustc_middle/src/values.rs index 8d614a535498f..bc73d36216ef4 100644 --- a/compiler/rustc_middle/src/values.rs +++ b/compiler/rustc_middle/src/values.rs @@ -88,7 +88,7 @@ impl<'tcx> Value> for Representability { if info.query.dep_kind == dep_kinds::representability && let Some(field_id) = info.query.def_id && let Some(field_id) = field_id.as_local() - && let Some(DefKind::Field) = info.query.def_kind + && let Some(DefKind::Field) = info.query.info.def_kind { let parent_id = tcx.parent(field_id.to_def_id()); let item_id = match tcx.def_kind(parent_id) { @@ -224,7 +224,7 @@ impl<'tcx, T> Value> for Result> continue; }; let frame_span = - frame.query.default_span(cycle[(i + 1) % cycle.len()].span); + frame.query.info.default_span(cycle[(i + 1) % cycle.len()].span); if frame_span.is_dummy() { continue; } diff --git a/compiler/rustc_query_impl/src/lib.rs b/compiler/rustc_query_impl/src/lib.rs index f763b707aa234..6904af771f0cc 100644 --- a/compiler/rustc_query_impl/src/lib.rs +++ b/compiler/rustc_query_impl/src/lib.rs @@ -21,8 +21,8 @@ use rustc_middle::ty::TyCtxt; use rustc_query_system::dep_graph::SerializedDepNodeIndex; use rustc_query_system::ich::StableHashingContext; use rustc_query_system::query::{ - CycleError, HashResult, QueryCache, QueryConfig, QueryMap, QueryMode, QueryState, - get_query_incr, get_query_non_incr, + CycleError, HashResult, QueryCache, QueryConfig, QueryMap, QueryMode, QueryStackDeferred, + QueryState, get_query_incr, get_query_non_incr, }; use rustc_query_system::{HandleCycleError, Value}; use rustc_span::{ErrorGuaranteed, Span}; @@ -79,7 +79,10 @@ where } #[inline(always)] - fn query_state<'a>(self, qcx: QueryCtxt<'tcx>) -> &'a QueryState + fn query_state<'a>( + self, + qcx: QueryCtxt<'tcx>, + ) -> &'a QueryState> where QueryCtxt<'tcx>: 'a, { @@ -88,7 +91,7 @@ where unsafe { &*(&qcx.tcx.query_system.states as *const QueryStates<'tcx>) .byte_add(self.dynamic.query_state) - .cast::>() + .cast::>>() } } diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 67ab1114af62b..c98affe0cb198 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -6,6 +6,7 @@ use std::num::NonZero; use rustc_data_structures::jobserver::Proxy; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_data_structures::sync::{DynSend, DynSync}; use rustc_data_structures::unord::UnordMap; use rustc_hashes::Hash64; use rustc_hir::limit::Limit; @@ -26,8 +27,8 @@ use rustc_middle::ty::{self, TyCtxt}; use rustc_query_system::dep_graph::{DepNodeParams, HasDepContext}; use rustc_query_system::ich::StableHashingContext; use rustc_query_system::query::{ - QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffect, QueryStackFrame, - force_query, + QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffect, + QueryStackDeferred, QueryStackFrame, QueryStackFrameExtra, force_query, }; use rustc_query_system::{QueryOverflow, QueryOverflowNote}; use rustc_serialize::{Decodable, Encodable}; @@ -66,7 +67,9 @@ impl<'tcx> HasDepContext for QueryCtxt<'tcx> { } } -impl QueryContext for QueryCtxt<'_> { +impl<'tcx> QueryContext for QueryCtxt<'tcx> { + type QueryInfo = QueryStackDeferred<'tcx>; + #[inline] fn jobserver_proxy(&self) -> &Proxy { &*self.jobserver_proxy @@ -95,7 +98,10 @@ impl QueryContext for QueryCtxt<'_> { /// Prefer passing `false` to `require_complete` to avoid potential deadlocks, /// especially when called from within a deadlock handler, unless a /// complete map is needed and no deadlock is possible at this call site. - fn collect_active_jobs(self, require_complete: bool) -> Result { + fn collect_active_jobs( + self, + require_complete: bool, + ) -> Result>, QueryMap>> { let mut jobs = QueryMap::default(); let mut complete = true; @@ -108,6 +114,13 @@ impl QueryContext for QueryCtxt<'_> { if complete { Ok(jobs) } else { Err(jobs) } } + fn lift_query_info( + self, + info: &QueryStackDeferred<'tcx>, + ) -> rustc_query_system::query::QueryStackFrameExtra { + info.extract() + } + // Interactions with on_disk_cache fn load_side_effect( self, @@ -168,7 +181,10 @@ impl QueryContext for QueryCtxt<'_> { self.sess.dcx().emit_fatal(QueryOverflow { span: info.job.span, - note: QueryOverflowNote { desc: info.query.description, depth }, + note: QueryOverflowNote { + desc: self.lift_query_info(&info.query.info).description, + depth, + }, suggested_limit, crate_name: self.crate_name(LOCAL_CRATE), }); @@ -305,16 +321,17 @@ macro_rules! should_ever_cache_on_disk { }; } -pub(crate) fn create_query_frame< - 'tcx, - K: Copy + Key + for<'a> HashStable>, ->( - tcx: TyCtxt<'tcx>, - do_describe: fn(TyCtxt<'tcx>, K) -> String, - key: K, - kind: DepKind, - name: &'static str, -) -> QueryStackFrame { +fn create_query_frame_extra<'tcx, K: Key + Copy + 'tcx>( + (tcx, key, kind, name, do_describe): ( + TyCtxt<'tcx>, + K, + DepKind, + &'static str, + fn(TyCtxt<'tcx>, K) -> String, + ), +) -> QueryStackFrameExtra { + let def_id = key.key_as_def_id(); + // If reduced queries are requested, we may be printing a query stack due // to a panic. Avoid using `default_span` and `def_kind` in that case. let reduce_queries = with_reduced_queries(); @@ -326,36 +343,49 @@ pub(crate) fn create_query_frame< } else { description }; - - let span = if reduce_queries { + let span = if kind == dep_graph::dep_kinds::def_span || reduce_queries { // The `def_span` query is used to calculate `default_span`, // so exit to avoid infinite recursion. None } else { - Some(tcx.with_reduced_queries(|| key.default_span(tcx))) + Some(key.default_span(tcx)) }; - let def_id = key.key_as_def_id(); - - let def_kind = if reduce_queries { + let def_kind = if kind == dep_graph::dep_kinds::def_kind || reduce_queries { // Try to avoid infinite recursion. None } else { - def_id - .and_then(|def_id| def_id.as_local()) - .map(|def_id| tcx.with_reduced_queries(|| tcx.def_kind(def_id))) + def_id.and_then(|def_id| def_id.as_local()).map(|def_id| tcx.def_kind(def_id)) }; + QueryStackFrameExtra::new(description, span, def_kind) +} + +pub(crate) fn create_query_frame< + 'tcx, + K: Copy + DynSend + DynSync + Key + for<'a> HashStable> + 'tcx, +>( + tcx: TyCtxt<'tcx>, + do_describe: fn(TyCtxt<'tcx>, K) -> String, + key: K, + kind: DepKind, + name: &'static str, +) -> QueryStackFrame> { + let def_id = key.key_as_def_id(); + let hash = || { + tcx.with_stable_hashing_context(|mut hcx| { + let mut hasher = StableHasher::new(); + kind.as_usize().hash_stable(&mut hcx, &mut hasher); + key.hash_stable(&mut hcx, &mut hasher); + hasher.finish::() + }) + }; let def_id_for_ty_in_cycle = key.def_id_for_ty_in_cycle(); - let hash = tcx.with_stable_hashing_context(|mut hcx| { - let mut hasher = StableHasher::new(); - kind.as_usize().hash_stable(&mut hcx, &mut hasher); - key.hash_stable(&mut hcx, &mut hasher); - hasher.finish::() - }); + let info = + QueryStackDeferred::new((tcx, key, kind, name, do_describe), create_query_frame_extra); - QueryStackFrame::new(description, span, def_id, def_kind, kind, def_id_for_ty_in_cycle, hash) + QueryStackFrame::new(info, kind, hash, def_id, def_id_for_ty_in_cycle) } pub(crate) fn encode_query_results<'a, 'tcx, Q>( @@ -707,7 +737,7 @@ macro_rules! define_queries { pub(crate) fn collect_active_jobs<'tcx>( tcx: TyCtxt<'tcx>, - qmap: &mut QueryMap, + qmap: &mut QueryMap>, require_complete: bool, ) -> Option<()> { let make_query = |tcx, key| { @@ -791,7 +821,7 @@ macro_rules! define_queries { // These arrays are used for iteration and can't be indexed by `DepKind`. const COLLECT_ACTIVE_JOBS: &[ - for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap, bool) -> Option<()> + for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap>, bool) -> Option<()> ] = &[$(query_impl::$name::collect_active_jobs),*]; diff --git a/compiler/rustc_query_system/src/query/config.rs b/compiler/rustc_query_system/src/query/config.rs index 371b896400a58..e508eadb73b0b 100644 --- a/compiler/rustc_query_system/src/query/config.rs +++ b/compiler/rustc_query_system/src/query/config.rs @@ -6,6 +6,7 @@ use std::hash::Hash; use rustc_data_structures::fingerprint::Fingerprint; use rustc_span::ErrorGuaranteed; +use super::QueryStackFrameExtra; use crate::dep_graph::{DepKind, DepNode, DepNodeParams, SerializedDepNodeIndex}; use crate::error::HandleCycleError; use crate::ich::StableHashingContext; @@ -27,7 +28,7 @@ pub trait QueryConfig: Copy { fn format_value(self) -> fn(&Self::Value) -> String; // Don't use this method to access query results, instead use the methods on TyCtxt - fn query_state<'a>(self, tcx: Qcx) -> &'a QueryState + fn query_state<'a>(self, tcx: Qcx) -> &'a QueryState where Qcx: 'a; @@ -57,7 +58,7 @@ pub trait QueryConfig: Copy { fn value_from_cycle_error( self, tcx: Qcx::DepContext, - cycle_error: &CycleError, + cycle_error: &CycleError, guar: ErrorGuaranteed, ) -> Self::Value; diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index 4a9b6a0d557cb..7d9b594d501ff 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -1,3 +1,4 @@ +use std::fmt::Debug; use std::hash::Hash; use std::io::Write; use std::iter; @@ -11,6 +12,7 @@ use rustc_hir::def::DefKind; use rustc_session::Session; use rustc_span::{DUMMY_SP, Span}; +use super::QueryStackFrameExtra; use crate::dep_graph::DepContext; use crate::error::CycleStack; use crate::query::plumbing::CycleError; @@ -18,45 +20,54 @@ use crate::query::{QueryContext, QueryStackFrame}; /// Represents a span and a query key. #[derive(Clone, Debug)] -pub struct QueryInfo { +pub struct QueryInfo { /// The span corresponding to the reason for which this query was required. pub span: Span, - pub query: QueryStackFrame, + pub query: QueryStackFrame, } -pub type QueryMap = FxHashMap; +impl QueryInfo { + pub(crate) fn lift>( + &self, + qcx: Qcx, + ) -> QueryInfo { + QueryInfo { span: self.span, query: self.query.lift(qcx) } + } +} + +pub type QueryMap = FxHashMap>; /// A value uniquely identifying an active query job. #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)] pub struct QueryJobId(pub NonZero); impl QueryJobId { - fn query(self, map: &QueryMap) -> QueryStackFrame { + fn query(self, map: &QueryMap) -> QueryStackFrame { map.get(&self).unwrap().query.clone() } - fn span(self, map: &QueryMap) -> Span { + fn span(self, map: &QueryMap) -> Span { map.get(&self).unwrap().job.span } - fn parent(self, map: &QueryMap) -> Option { + fn parent(self, map: &QueryMap) -> Option { map.get(&self).unwrap().job.parent } - fn latch(self, map: &QueryMap) -> Option<&QueryLatch> { + fn latch(self, map: &QueryMap) -> Option<&QueryLatch> { map.get(&self).unwrap().job.latch.as_ref() } } #[derive(Clone, Debug)] -pub struct QueryJobInfo { - pub query: QueryStackFrame, - pub job: QueryJob, +pub struct QueryJobInfo { + pub query: QueryStackFrame, + pub job: QueryJob, } /// Represents an active query job. #[derive(Debug)] -pub struct QueryJob { +pub struct QueryJob { pub id: QueryJobId, /// The span corresponding to the reason for which this query was required. @@ -66,23 +77,23 @@ pub struct QueryJob { pub parent: Option, /// The latch that is used to wait on this job. - latch: Option, + latch: Option>, } -impl Clone for QueryJob { +impl Clone for QueryJob { fn clone(&self) -> Self { Self { id: self.id, span: self.span, parent: self.parent, latch: self.latch.clone() } } } -impl QueryJob { +impl QueryJob { /// Creates a new query job. #[inline] pub fn new(id: QueryJobId, span: Span, parent: Option) -> Self { QueryJob { id, span, parent, latch: None } } - pub(super) fn latch(&mut self) -> QueryLatch { + pub(super) fn latch(&mut self) -> QueryLatch { if self.latch.is_none() { self.latch = Some(QueryLatch::new()); } @@ -102,12 +113,12 @@ impl QueryJob { } impl QueryJobId { - pub(super) fn find_cycle_in_stack( + pub(super) fn find_cycle_in_stack( &self, - query_map: QueryMap, + query_map: QueryMap, current_job: &Option, span: Span, - ) -> CycleError { + ) -> CycleError { // Find the waitee amongst `current_job` parents let mut cycle = Vec::new(); let mut current_job = Option::clone(current_job); @@ -141,7 +152,7 @@ impl QueryJobId { #[cold] #[inline(never)] - pub fn find_dep_kind_root(&self, query_map: QueryMap) -> (QueryJobInfo, usize) { + pub fn find_dep_kind_root(&self, query_map: QueryMap) -> (QueryJobInfo, usize) { let mut depth = 1; let info = query_map.get(&self).unwrap(); let dep_kind = info.query.dep_kind; @@ -161,31 +172,31 @@ impl QueryJobId { } #[derive(Debug)] -struct QueryWaiter { +struct QueryWaiter { query: Option, condvar: Condvar, span: Span, - cycle: Mutex>, + cycle: Mutex>>, } #[derive(Debug)] -struct QueryLatchInfo { +struct QueryLatchInfo { complete: bool, - waiters: Vec>, + waiters: Vec>>, } #[derive(Debug)] -pub(super) struct QueryLatch { - info: Arc>, +pub(super) struct QueryLatch { + info: Arc>>, } -impl Clone for QueryLatch { +impl Clone for QueryLatch { fn clone(&self) -> Self { Self { info: Arc::clone(&self.info) } } } -impl QueryLatch { +impl QueryLatch { fn new() -> Self { QueryLatch { info: Arc::new(Mutex::new(QueryLatchInfo { complete: false, waiters: Vec::new() })), @@ -198,7 +209,7 @@ impl QueryLatch { qcx: impl QueryContext, query: Option, span: Span, - ) -> Result<(), CycleError> { + ) -> Result<(), CycleError> { let waiter = Arc::new(QueryWaiter { query, span, cycle: Mutex::new(None), condvar: Condvar::new() }); self.wait_on_inner(qcx, &waiter); @@ -213,7 +224,7 @@ impl QueryLatch { } /// Awaits the caller on this latch by blocking the current thread. - fn wait_on_inner(&self, qcx: impl QueryContext, waiter: &Arc) { + fn wait_on_inner(&self, qcx: impl QueryContext, waiter: &Arc>) { let mut info = self.info.lock(); if !info.complete { // We push the waiter on to the `waiters` list. It can be accessed inside @@ -249,7 +260,7 @@ impl QueryLatch { /// Removes a single waiter from the list of waiters. /// This is used to break query cycles. - fn extract_waiter(&self, waiter: usize) -> Arc { + fn extract_waiter(&self, waiter: usize) -> Arc> { let mut info = self.info.lock(); debug_assert!(!info.complete); // Remove the waiter from the list of waiters @@ -269,7 +280,11 @@ type Waiter = (QueryJobId, usize); /// For visits of resumable waiters it returns Some(Some(Waiter)) which has the /// required information to resume the waiter. /// If all `visit` calls returns None, this function also returns None. -fn visit_waiters(query_map: &QueryMap, query: QueryJobId, mut visit: F) -> Option> +fn visit_waiters( + query_map: &QueryMap, + query: QueryJobId, + mut visit: F, +) -> Option> where F: FnMut(Span, QueryJobId) -> Option>, { @@ -299,8 +314,8 @@ where /// `span` is the reason for the `query` to execute. This is initially DUMMY_SP. /// If a cycle is detected, this initial value is replaced with the span causing /// the cycle. -fn cycle_check( - query_map: &QueryMap, +fn cycle_check( + query_map: &QueryMap, query: QueryJobId, span: Span, stack: &mut Vec<(Span, QueryJobId)>, @@ -339,8 +354,8 @@ fn cycle_check( /// Finds out if there's a path to the compiler root (aka. code which isn't in a query) /// from `query` without going through any of the queries in `visited`. /// This is achieved with a depth first search. -fn connected_to_root( - query_map: &QueryMap, +fn connected_to_root( + query_map: &QueryMap, query: QueryJobId, visited: &mut FxHashSet, ) -> bool { @@ -361,7 +376,7 @@ fn connected_to_root( } // Deterministically pick an query from a list -fn pick_query<'a, T, F>(query_map: &QueryMap, queries: &'a [T], f: F) -> &'a T +fn pick_query<'a, I: Clone, T, F>(query_map: &QueryMap, queries: &'a [T], f: F) -> &'a T where F: Fn(&T) -> (Span, QueryJobId), { @@ -386,10 +401,10 @@ where /// the function return true. /// If a cycle was not found, the starting query is removed from `jobs` and /// the function returns false. -fn remove_cycle( - query_map: &QueryMap, +fn remove_cycle( + query_map: &QueryMap, jobs: &mut Vec, - wakelist: &mut Vec>, + wakelist: &mut Vec>>, ) -> bool { let mut visited = FxHashSet::default(); let mut stack = Vec::new(); @@ -490,7 +505,10 @@ fn remove_cycle( /// uses a query latch and then resuming that waiter. /// There may be multiple cycles involved in a deadlock, so this searches /// all active queries for cycles before finally resuming all the waiters at once. -pub fn break_query_cycles(query_map: QueryMap, registry: &rustc_thread_pool::Registry) { +pub fn break_query_cycles( + query_map: QueryMap, + registry: &rustc_thread_pool::Registry, +) { let mut wakelist = Vec::new(); // It is OK per the comments: // - https://github.com/rust-lang/rust/pull/131200#issuecomment-2798854932 @@ -541,7 +559,7 @@ pub fn report_cycle<'a>( ) -> Diag<'a> { assert!(!stack.is_empty()); - let span = stack[0].query.default_span(stack[1 % stack.len()].span); + let span = stack[0].query.info.default_span(stack[1 % stack.len()].span); let mut cycle_stack = Vec::new(); @@ -550,31 +568,31 @@ pub fn report_cycle<'a>( for i in 1..stack.len() { let query = &stack[i].query; - let span = query.default_span(stack[(i + 1) % stack.len()].span); - cycle_stack.push(CycleStack { span, desc: query.description.to_owned() }); + let span = query.info.default_span(stack[(i + 1) % stack.len()].span); + cycle_stack.push(CycleStack { span, desc: query.info.description.to_owned() }); } let mut cycle_usage = None; if let Some((span, ref query)) = *usage { cycle_usage = Some(crate::error::CycleUsage { - span: query.default_span(span), - usage: query.description.to_string(), + span: query.info.default_span(span), + usage: query.info.description.to_string(), }); } - let alias = if stack.iter().all(|entry| matches!(entry.query.def_kind, Some(DefKind::TyAlias))) - { - Some(crate::error::Alias::Ty) - } else if stack.iter().all(|entry| entry.query.def_kind == Some(DefKind::TraitAlias)) { - Some(crate::error::Alias::Trait) - } else { - None - }; + let alias = + if stack.iter().all(|entry| matches!(entry.query.info.def_kind, Some(DefKind::TyAlias))) { + Some(crate::error::Alias::Ty) + } else if stack.iter().all(|entry| entry.query.info.def_kind == Some(DefKind::TraitAlias)) { + Some(crate::error::Alias::Trait) + } else { + None + }; let cycle_diag = crate::error::Cycle { span, cycle_stack, - stack_bottom: stack[0].query.description.to_owned(), + stack_bottom: stack[0].query.info.description.to_owned(), alias, cycle_usage, stack_count, @@ -610,6 +628,7 @@ pub fn print_query_stack( let Some(query_info) = query_map.get(&query) else { break; }; + let query_extra = qcx.lift_query_info(&query_info.query.info); if Some(count_printed) < limit_frames || limit_frames.is_none() { // Only print to stderr as many stack frames as `num_frames` when present. // FIXME: needs translation @@ -617,7 +636,7 @@ pub fn print_query_stack( #[allow(rustc::untranslatable_diagnostic)] dcx.struct_failure_note(format!( "#{} [{:?}] {}", - count_printed, query_info.query.dep_kind, query_info.query.description + count_printed, query_info.query.dep_kind, query_extra.description )) .with_span(query_info.job.span) .emit(); @@ -630,7 +649,7 @@ pub fn print_query_stack( "#{} [{}] {}", count_total, qcx.dep_context().dep_kind_info(query_info.query.dep_kind).name, - query_info.query.description + query_extra.description ); } diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index b524756d81b61..ce3456d532e69 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -1,4 +1,23 @@ +mod plumbing; +use std::fmt::Debug; +use std::marker::PhantomData; +use std::mem::transmute; +use std::sync::Arc; + +pub use self::plumbing::*; + +mod job; +pub use self::job::{ + QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryMap, break_query_cycles, print_query_stack, + report_cycle, +}; + +mod caches; +pub use self::caches::{DefIdCache, DefaultCache, QueryCache, SingleCache, VecCache}; + +mod config; use rustc_data_structures::jobserver::Proxy; +use rustc_data_structures::sync::{DynSend, DynSync}; use rustc_errors::DiagInner; use rustc_hashes::Hash64; use rustc_hir::def::DefKind; @@ -6,49 +25,66 @@ use rustc_macros::{Decodable, Encodable}; use rustc_span::Span; use rustc_span::def_id::DefId; -pub use self::caches::{DefIdCache, DefaultCache, QueryCache, SingleCache, VecCache}; pub use self::config::{HashResult, QueryConfig}; -pub use self::job::{ - QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryMap, break_query_cycles, print_query_stack, - report_cycle, -}; -pub use self::plumbing::*; use crate::dep_graph::{DepKind, DepNodeIndex, HasDepContext, SerializedDepNodeIndex}; -mod caches; -mod config; -mod job; -mod plumbing; - /// Description of a frame in the query stack. /// /// This is mostly used in case of cycles for error reporting. #[derive(Clone, Debug)] -pub struct QueryStackFrame { - pub description: String, - span: Option, - pub def_id: Option, - pub def_kind: Option, - /// A def-id that is extracted from a `Ty` in a query key - pub def_id_for_ty_in_cycle: Option, +pub struct QueryStackFrame { + /// This field initially stores a `QueryStackDeferred` during collection, + /// but can later be changed to `QueryStackFrameExtra` containing concrete information + /// by calling `lift`. This is done so that collecting query does not need to invoke + /// queries, instead `lift` will call queries in a more appropriate location. + pub info: I, + pub dep_kind: DepKind, /// This hash is used to deterministically pick /// a query to remove cycles in the parallel compiler. hash: Hash64, + pub def_id: Option, + /// A def-id that is extracted from a `Ty` in a query key + pub def_id_for_ty_in_cycle: Option, } -impl QueryStackFrame { +impl QueryStackFrame { #[inline] pub fn new( - description: String, - span: Option, - def_id: Option, - def_kind: Option, + info: I, dep_kind: DepKind, + hash: impl FnOnce() -> Hash64, + def_id: Option, def_id_for_ty_in_cycle: Option, - hash: Hash64, ) -> Self { - Self { description, span, def_id, def_kind, def_id_for_ty_in_cycle, dep_kind, hash } + Self { info, def_id, dep_kind, hash: hash(), def_id_for_ty_in_cycle } + } + + fn lift>( + &self, + qcx: Qcx, + ) -> QueryStackFrame { + QueryStackFrame { + info: qcx.lift_query_info(&self.info), + dep_kind: self.dep_kind, + hash: self.hash, + def_id: self.def_id, + def_id_for_ty_in_cycle: self.def_id_for_ty_in_cycle, + } + } +} + +#[derive(Clone, Debug)] +pub struct QueryStackFrameExtra { + pub description: String, + span: Option, + pub def_kind: Option, +} + +impl QueryStackFrameExtra { + #[inline] + pub fn new(description: String, span: Option, def_kind: Option) -> Self { + Self { description, span, def_kind } } // FIXME(eddyb) Get more valid `Span`s on queries. @@ -61,6 +97,40 @@ impl QueryStackFrame { } } +/// Track a 'side effect' for a particular query. +/// This is used to hold a closure which can create `QueryStackFrameExtra`. +#[derive(Clone)] +pub struct QueryStackDeferred<'tcx> { + _dummy: PhantomData<&'tcx ()>, + + // `extract` may contain references to 'tcx, but we can't tell drop checking that it won't + // access it in the destructor. + extract: Arc QueryStackFrameExtra + DynSync + DynSend>, +} + +impl<'tcx> QueryStackDeferred<'tcx> { + pub fn new( + context: C, + extract: fn(C) -> QueryStackFrameExtra, + ) -> Self { + let extract: Arc QueryStackFrameExtra + DynSync + DynSend + 'tcx> = + Arc::new(move || extract(context)); + // SAFETY: The `extract` closure does not access 'tcx in its destructor as the only + // captured variable is `context` which is Copy and cannot have a destructor. + Self { _dummy: PhantomData, extract: unsafe { transmute(extract) } } + } + + pub fn extract(&self) -> QueryStackFrameExtra { + (self.extract)() + } +} + +impl<'tcx> Debug for QueryStackDeferred<'tcx> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("QueryStackDeferred") + } +} + /// Tracks 'side effects' for a particular query. /// This struct is saved to disk along with the query result, /// and loaded from disk if we mark the query as green. @@ -80,6 +150,8 @@ pub enum QuerySideEffect { } pub trait QueryContext: HasDepContext { + type QueryInfo: Clone; + /// Gets a jobserver reference which is used to release then acquire /// a token while waiting on a query. fn jobserver_proxy(&self) -> &Proxy; @@ -89,7 +161,12 @@ pub trait QueryContext: HasDepContext { /// Get the query information from the TLS context. fn current_query_job(self) -> Option; - fn collect_active_jobs(self, require_complete: bool) -> Result; + fn collect_active_jobs( + self, + require_complete: bool, + ) -> Result, QueryMap>; + + fn lift_query_info(self, info: &Self::QueryInfo) -> QueryStackFrameExtra; /// Load a side effect associated to the node in the previous session. fn load_side_effect( diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index fa5a94d651885..dea47c8fa787e 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -18,7 +18,7 @@ use rustc_errors::{Diag, FatalError, StashKey}; use rustc_span::{DUMMY_SP, Span}; use tracing::instrument; -use super::QueryConfig; +use super::{QueryConfig, QueryStackFrameExtra}; use crate::HandleCycleError; use crate::dep_graph::{DepContext, DepGraphData, DepNode, DepNodeIndex, DepNodeParams}; use crate::ich::StableHashingContext; @@ -31,23 +31,23 @@ fn equivalent_key(k: &K) -> impl Fn(&(K, V)) -> bool + '_ { move |x| x.0 == *k } -pub struct QueryState { - active: Sharded>, +pub struct QueryState { + active: Sharded)>>, } /// Indicates the state of a query for a given key in a query map. -enum QueryResult { +enum QueryResult { /// An already executing query. The query job can be used to await for its completion. - Started(QueryJob), + Started(QueryJob), /// The query panicked. Queries trying to wait on this will raise a fatal error which will /// silently panic. Poisoned, } -impl QueryResult { +impl QueryResult { /// Unwraps the query job expecting that it has started. - fn expect_job(self) -> QueryJob { + fn expect_job(self) -> QueryJob { match self { Self::Started(job) => job, Self::Poisoned => { @@ -57,7 +57,7 @@ impl QueryResult { } } -impl QueryState +impl QueryState where K: Eq + Hash + Copy + Debug, { @@ -68,13 +68,13 @@ where pub fn collect_active_jobs( &self, qcx: Qcx, - make_query: fn(Qcx, K) -> QueryStackFrame, - jobs: &mut QueryMap, + make_query: fn(Qcx, K) -> QueryStackFrame, + jobs: &mut QueryMap, require_complete: bool, ) -> Option<()> { let mut active = Vec::new(); - let mut collect = |iter: LockGuard<'_, HashTable<(K, QueryResult)>>| { + let mut collect = |iter: LockGuard<'_, HashTable<(K, QueryResult)>>| { for (k, v) in iter.iter() { if let QueryResult::Started(ref job) = *v { active.push((*k, job.clone())); @@ -105,19 +105,19 @@ where } } -impl Default for QueryState { - fn default() -> QueryState { +impl Default for QueryState { + fn default() -> QueryState { QueryState { active: Default::default() } } } /// A type representing the responsibility to execute the job in the `job` field. /// This will poison the relevant query if dropped. -struct JobOwner<'tcx, K> +struct JobOwner<'tcx, K, I> where K: Eq + Hash + Copy, { - state: &'tcx QueryState, + state: &'tcx QueryState, key: K, } @@ -159,7 +159,7 @@ where } Stash => { let guar = if let Some(root) = cycle_error.cycle.first() - && let Some(span) = root.query.span + && let Some(span) = root.query.info.span { error.stash(span, StashKey::Cycle).unwrap() } else { @@ -170,7 +170,7 @@ where } } -impl<'tcx, K> JobOwner<'tcx, K> +impl<'tcx, K, I> JobOwner<'tcx, K, I> where K: Eq + Hash + Copy, { @@ -207,7 +207,7 @@ where } } -impl<'tcx, K> Drop for JobOwner<'tcx, K> +impl<'tcx, K, I> Drop for JobOwner<'tcx, K, I> where K: Eq + Hash + Copy, { @@ -235,10 +235,19 @@ where } #[derive(Clone, Debug)] -pub struct CycleError { +pub struct CycleError { /// The query and related span that uses the cycle. - pub usage: Option<(Span, QueryStackFrame)>, - pub cycle: Vec, + pub usage: Option<(Span, QueryStackFrame)>, + pub cycle: Vec>, +} + +impl CycleError { + fn lift>(&self, qcx: Qcx) -> CycleError { + CycleError { + usage: self.usage.as_ref().map(|(span, frame)| (*span, frame.lift(qcx))), + cycle: self.cycle.iter().map(|info| info.lift(qcx)).collect(), + } + } } /// Checks whether there is already a value for this key in the in-memory @@ -275,10 +284,10 @@ where { // Ensure there was no errors collecting all active jobs. // We need the complete map to ensure we find a cycle to break. - let query_map = qcx.collect_active_jobs(false).expect("failed to collect active queries"); + let query_map = qcx.collect_active_jobs(false).ok().expect("failed to collect active queries"); let error = try_execute.find_cycle_in_stack(query_map, &qcx.current_query_job(), span); - (mk_cycle(query, qcx, error), None) + (mk_cycle(query, qcx, error.lift(qcx)), None) } #[inline(always)] @@ -287,7 +296,7 @@ fn wait_for_query( qcx: Qcx, span: Span, key: Q::Key, - latch: QueryLatch, + latch: QueryLatch, current: Option, ) -> (Q::Value, Option) where @@ -327,7 +336,7 @@ where (v, Some(index)) } - Err(cycle) => (mk_cycle(query, qcx, cycle), None), + Err(cycle) => (mk_cycle(query, qcx, cycle.lift(qcx)), None), } } @@ -405,7 +414,7 @@ where fn execute_job( query: Q, qcx: Qcx, - state: &QueryState, + state: &QueryState, key: Q::Key, key_hash: u64, id: QueryJobId, diff --git a/tests/ui/query-system/query-cycle-printing-issue-151226.rs b/tests/ui/query-system/query-cycle-printing-issue-151226.rs new file mode 100644 index 0000000000000..9d0a20737c9fa --- /dev/null +++ b/tests/ui/query-system/query-cycle-printing-issue-151226.rs @@ -0,0 +1,8 @@ +struct A(std::sync::OnceLock); +//~^ ERROR recursive type `A` has infinite size +//~| ERROR cycle detected when computing layout of `A<()>` + +static B: A<()> = todo!(); +//~^ ERROR cycle occurred during layout computation + +fn main() {} diff --git a/tests/ui/query-system/query-cycle-printing-issue-151226.stderr b/tests/ui/query-system/query-cycle-printing-issue-151226.stderr new file mode 100644 index 0000000000000..7e574b5911a39 --- /dev/null +++ b/tests/ui/query-system/query-cycle-printing-issue-151226.stderr @@ -0,0 +1,36 @@ +error[E0072]: recursive type `A` has infinite size + --> $DIR/query-cycle-printing-issue-151226.rs:1:1 + | +LL | struct A(std::sync::OnceLock); + | ^^^^^^^^^^^ ------------------------- recursive without indirection + | +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle + | +LL | struct A(Box>); + | ++++ + + +error[E0391]: cycle detected when computing layout of `A<()>` + | + = note: ...which requires computing layout of `std::sync::once_lock::OnceLock>`... + = note: ...which requires computing layout of `core::cell::UnsafeCell>>`... + = note: ...which requires computing layout of `core::mem::maybe_uninit::MaybeUninit>`... + = note: ...which requires computing layout of `core::mem::manually_drop::ManuallyDrop>`... + = note: ...which requires computing layout of `core::mem::maybe_dangling::MaybeDangling>`... + = note: ...which again requires computing layout of `A<()>`, completing the cycle +note: cycle used when checking that `B` is well-formed + --> $DIR/query-cycle-printing-issue-151226.rs:5:1 + | +LL | static B: A<()> = todo!(); + | ^^^^^^^^^^^^^^^ + = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information + +error[E0080]: a cycle occurred during layout computation + --> $DIR/query-cycle-printing-issue-151226.rs:5:1 + | +LL | static B: A<()> = todo!(); + | ^^^^^^^^^^^^^^^ evaluation of `B` failed here + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0072, E0080, E0391. +For more information about an error, try `rustc --explain E0072`. diff --git a/tests/ui/query-system/query-cycle-printing-issue-151358.rs b/tests/ui/query-system/query-cycle-printing-issue-151358.rs new file mode 100644 index 0000000000000..04d8664420be8 --- /dev/null +++ b/tests/ui/query-system/query-cycle-printing-issue-151358.rs @@ -0,0 +1,7 @@ +//~ ERROR: cycle detected when looking up span for `Default` +trait Default {} +use std::num::NonZero; +fn main() { + NonZero(); + format!("{}", 0); +} diff --git a/tests/ui/query-system/query-cycle-printing-issue-151358.stderr b/tests/ui/query-system/query-cycle-printing-issue-151358.stderr new file mode 100644 index 0000000000000..9c1d7b1de33a5 --- /dev/null +++ b/tests/ui/query-system/query-cycle-printing-issue-151358.stderr @@ -0,0 +1,9 @@ +error[E0391]: cycle detected when looking up span for `Default` + | + = note: ...which immediately requires looking up span for `Default` again + = note: cycle used when perform lints prior to AST lowering + = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0391`. diff --git a/tests/ui/resolve/query-cycle-issue-124901.rs b/tests/ui/resolve/query-cycle-issue-124901.rs index ccaee0e6bc6fd..6cb1e58b6258f 100644 --- a/tests/ui/resolve/query-cycle-issue-124901.rs +++ b/tests/ui/resolve/query-cycle-issue-124901.rs @@ -1,4 +1,4 @@ -//~ ERROR: cycle detected when getting HIR ID of `Default` +//~ ERROR: cycle detected when looking up span for `Default` trait Default { type Id; diff --git a/tests/ui/resolve/query-cycle-issue-124901.stderr b/tests/ui/resolve/query-cycle-issue-124901.stderr index 3679925c6db42..9c1d7b1de33a5 100644 --- a/tests/ui/resolve/query-cycle-issue-124901.stderr +++ b/tests/ui/resolve/query-cycle-issue-124901.stderr @@ -1,10 +1,7 @@ -error[E0391]: cycle detected when getting HIR ID of `Default` +error[E0391]: cycle detected when looking up span for `Default` | - = note: ...which requires getting the crate HIR... - = note: ...which requires perform lints prior to AST lowering... - = note: ...which requires looking up span for `Default`... - = note: ...which again requires getting HIR ID of `Default`, completing the cycle - = note: cycle used when getting the resolver for lowering + = note: ...which immediately requires looking up span for `Default` again + = note: cycle used when perform lints prior to AST lowering = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information error: aborting due to 1 previous error