use rustc_data_structures::bit_set::GrowableBitSet;
use rustc_data_structures::sync::Lock;
use rustc_target::spec::abi::Abi;
+use std::cell::Cell;
use std::cmp;
use std::fmt::{self, Display};
use std::iter;
/// selection-context's freshener. Used to check for recursion.
fresh_trait_ref: ty::PolyTraitRef<'tcx>,
+ /// Starts out as false -- if, during evaluation, we encounter a
+ /// cycle, then we will set this flag to true for all participants
+ /// in the cycle (apart from the "head" node). These participants
+ /// will then forego caching their results. This is not the most
+ /// efficient solution, but it addresses #60010. The problem we
+ /// are trying to prevent:
+ ///
+ /// - If you have `A: AutoTrait` requires `B: AutoTrait` and `C: NonAutoTrait`
+ /// - `B: AutoTrait` requires `A: AutoTrait` (coinductive cycle, ok)
+ /// - `C: NonAutoTrait` requires `A: AutoTrait` (non-coinductive cycle, not ok)
+ ///
+ /// you don't want to cache that `B: AutoTrait` or `A: AutoTrait`
+ /// is `EvaluatedToOk`; this is because they were only considered
+ /// ok on the premise that if `A: AutoTrait` held, but we indeed
+ /// encountered a problem (later on) with `A: AutoTrait. So we
+ /// currently set a flag on the stack node for `B: AutoTrait` (as
+ /// well as the second instance of `A: AutoTrait`) to supress
+ /// caching.
+ ///
+ /// This is a simple, targeted fix. The correct fix requires
+ /// deeper changes, but would permit more caching: we could
+ /// basically defer caching until we have fully evaluated the
+ /// tree, and then cache the entire tree at once.
+ in_cycle: Cell<bool>,
+
previous: TraitObligationStackList<'prev, 'tcx>,
}
let (result, dep_node) = self.in_task(|this| this.evaluate_stack(&stack));
let result = result?;
- debug!("CACHE MISS: EVAL({:?})={:?}", fresh_trait_ref, result);
- self.insert_evaluation_cache(obligation.param_env, fresh_trait_ref, dep_node, result);
+ if !stack.in_cycle.get() {
+ debug!("CACHE MISS: EVAL({:?})={:?}", fresh_trait_ref, result);
+ self.insert_evaluation_cache(obligation.param_env, fresh_trait_ref, dep_node, result);
+ } else {
+ debug!(
+ "evaluate_trait_predicate_recursively: skipping cache because {:?} \
+ is a cycle participant",
+ fresh_trait_ref,
+ );
+ }
Ok(result)
}
{
debug!("evaluate_stack({:?}) --> recursive", stack.fresh_trait_ref);
+ // If we have a stack like `A B C D E A`, where the top of
+ // the stack is the final `A`, then this will iterate over
+ // `A, E, D, C, B` -- i.e., all the participants apart
+ // from the cycle head. We mark them as participating in a
+ // cycle. This suppresses caching for those nodes. See
+ // `in_cycle` field for more details.
+ for item in stack.iter().take(rec_index + 1) {
+ debug!("evaluate_stack: marking {:?} as cycle participant", item.fresh_trait_ref);
+ item.in_cycle.set(true);
+ }
+
// Subtle: when checking for a coinductive cycle, we do
// not compare using the "freshened trait refs" (which
// have erased regions) but rather the fully explicit
TraitObligationStack {
obligation,
fresh_trait_ref,
+ in_cycle: Cell::new(false),
previous: previous_stack,
}
}
--- /dev/null
+// Test that we properly detect the cycle amongst the traits
+// here and report an error.
+
+use std::panic::RefUnwindSafe;
+
+trait Database {
+ type Storage;
+}
+trait HasQueryGroup {}
+trait Query<DB> {
+ type Data;
+}
+trait SourceDatabase {
+ fn parse(&self) {
+ loop {}
+ }
+}
+
+struct ParseQuery;
+struct RootDatabase {
+ _runtime: Runtime<RootDatabase>,
+}
+struct Runtime<DB: Database> {
+ _storage: Box<DB::Storage>,
+}
+struct SalsaStorage {
+ _parse: <ParseQuery as Query<RootDatabase>>::Data, //~ ERROR overflow
+}
+
+impl Database for RootDatabase { //~ ERROR overflow
+ type Storage = SalsaStorage;
+}
+impl HasQueryGroup for RootDatabase {}
+impl<DB> Query<DB> for ParseQuery
+where
+ DB: SourceDatabase,
+ DB: Database,
+{
+ type Data = RootDatabase;
+}
+impl<T> SourceDatabase for T
+where
+ T: RefUnwindSafe,
+ T: HasQueryGroup,
+{
+}
+
+pub(crate) fn goto_implementation(db: &RootDatabase) -> u32 {
+ // This is not satisfied:
+ //
+ // - `RootDatabase: SourceDatabase`
+ // - requires `RootDatabase: RefUnwindSafe` + `RootDatabase: HasQueryGroup`
+ // - `RootDatabase: RefUnwindSafe`
+ // - requires `Runtime<RootDatabase>: RefUnwindSafe`
+ // - `Runtime<RootDatabase>: RefUnwindSafe`
+ // - requires `DB::Storage: RefUnwindSafe` (`SalsaStorage: RefUnwindSafe`)
+ // - `SalsaStorage: RefUnwindSafe`
+ // - requires `<ParseQuery as Query<RootDatabase>>::Data: RefUnwindSafe`,
+ // which means `ParseQuery: Query<RootDatabase>`
+ // - `ParseQuery: Query<RootDatabase>`
+ // - requires `RootDatabase: SourceDatabase`,
+ // - `RootDatabase: SourceDatabase` is already on the stack, so we have a
+ // cycle with non-coinductive participants
+ //
+ // we used to fail to report an error here because we got the
+ // caching wrong.
+ SourceDatabase::parse(db);
+ 22
+}
+
+fn main() {}
--- /dev/null
+error[E0275]: overflow evaluating the requirement `RootDatabase: SourceDatabase`
+ --> $DIR/cycle-cache-err-60010.rs:27:5
+ |
+LL | _parse: <ParseQuery as Query<RootDatabase>>::Data,
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = note: required because of the requirements on the impl of `Query<RootDatabase>` for `ParseQuery`
+
+error[E0275]: overflow evaluating the requirement `RootDatabase: SourceDatabase`
+ --> $DIR/cycle-cache-err-60010.rs:30:6
+ |
+LL | impl Database for RootDatabase {
+ | ^^^^^^^^
+ |
+ = note: required because of the requirements on the impl of `Query<RootDatabase>` for `ParseQuery`
+ = note: required because it appears within the type `SalsaStorage`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0275`.
projects / rust.git / commitdiff