Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ pub mod hardwired {
MUST_NOT_SUSPEND,
NAMED_ARGUMENTS_USED_POSITIONALLY,
NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE,
NEXT_TRAIT_SOLVER_OVERFLOW,
NON_CONTIGUOUS_RANGE_ENDPOINTS,
NON_EXHAUSTIVE_OMITTED_PATTERNS,
OUT_OF_SCOPE_MACRO_CALLS,
Expand Down Expand Up @@ -5579,3 +5580,65 @@ declare_lint! {
report_in_deps: true,
};
}

declare_lint! {
/// The `next_trait_solver_overflow` lint detects situations where the obligation evaluation
/// overflows with the next solver but not with the old solver.
///
/// ### Example
/// ```text
/// rustc -Znext-solver example.rs
/// ```
///
/// ```rust,ignore (requires next solver)
/// #![recursion_limit = "8"]
/// struct Foo<T> {
/// t: T,
/// opt_t: Option<T>,
/// }
/// fn require_sync<T: Sync>() {}
/// fn main() {
/// require_sync::<Foo<Foo<Foo<Foo<Foo<Foo<()>>>>>>>();
/// }
/// ```
///
/// This will produces:
/// ```text
/// error[E0275]: overflow evaluating the requirement `Foo<Foo<Foo<Foo<Foo<Foo<()>>>>>>: Sync`
/// --> example.rs:12:20
/// |
/// | require_sync::<Foo<Foo<Foo<Foo<Foo<Foo<()>>>>>>>();
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/// |
/// = help: consider increasing the recursion limit by adding a `#![recursion_limit = "16"]` attribute to your crate
/// note: required by a bound in `require_sync`
/// --> example.rs:9:20
/// |
/// | fn require_sync<T: Sync>() {}
/// | ^^^^ required by this bound in `require_sync`
/// ```
///
/// ### Explanation
///
/// The trait solvers use a recursion limit to avoid hangs from deeply nested obligations.
/// They also use caches to avoid redundant computation. This is a performance optimization and
/// shouldn't affect the final evaluation result.
///
/// However, the old solver doesn't validate depth requirement when looking up cache. This means
/// evaluation results depend on whether cache entries exists which in turn depends on cache
/// insertion order.
///
/// The next solver correctly records and validates recursion depth requirements when using
/// the cache. This makes it more prone to overflow compared to the old solver.
///
/// This is a [future-incompatible] lint to transition this to a hard error in the future.
///
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub NEXT_TRAIT_SOLVER_OVERFLOW,
Warn,
"detects trait solving overflow that only happens with the next solver",
@future_incompatible = FutureIncompatibleInfo {
reason: fcw!(FutureReleaseError #159228),
report_in_deps: false,
};
}
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2597,10 +2597,10 @@ rustc_queries! {

/// Used by `-Znext-solver` to compute proof trees.
query evaluate_root_goal_for_proof_tree_raw(
goal: solve::CanonicalInput<'tcx>,
key: (solve::CanonicalInput<'tcx>, usize)
) -> (solve::QueryResult<'tcx>, &'tcx solve::inspect::Probe<TyCtxt<'tcx>>) {
no_hash
desc { "computing proof tree for `{}`", goal.canonical.value.goal.predicate }
desc { "computing proof tree for `{}` with depth `{}`", key.0.canonical.value.goal.predicate, key.1 }
}

/// Returns the Rust target features for the current target. These are not always the same as LLVM target features!
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/query/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,12 @@ impl<'tcx, T: QueryKeyBounds> QueryKey for (CanonicalQueryInput<'tcx, T>, bool)
}
}

impl<'tcx, T: QueryKeyBounds> QueryKey for (CanonicalQueryInput<'tcx, T>, usize) {
fn default_span(&self, _tcx: TyCtxt<'_>) -> Span {
DUMMY_SP
}
}

impl<'tcx> QueryKey for (Ty<'tcx>, rustc_abi::VariantIdx) {
fn default_span(&self, _tcx: TyCtxt<'_>) -> Span {
DUMMY_SP
Expand Down
22 changes: 21 additions & 1 deletion compiler/rustc_middle/src/ty/context/impl_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{debug_assert_matches, fmt};

use rustc_errors::ErrorGuaranteed;
use rustc_hir as hir;
use rustc_hir::CRATE_HIR_ID;
use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::lang_items::LangItem;
Expand Down Expand Up @@ -786,8 +787,27 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
fn evaluate_root_goal_for_proof_tree_raw(
self,
canonical_goal: CanonicalInput<'tcx>,
root_depth: usize,
) -> (QueryResult<'tcx>, &'tcx inspect::Probe<TyCtxt<'tcx>>) {
self.evaluate_root_goal_for_proof_tree_raw(canonical_goal)
self.evaluate_root_goal_for_proof_tree_raw((canonical_goal, root_depth))
}

fn emit_next_solver_overflow_fcw(self, span: Option<Span>) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take a Span and supply a DUMMY_SP in the caller where necessary

self.emit_node_span_lint(
rustc_session::lint::builtin::NEXT_TRAIT_SOLVER_OVERFLOW,
CRATE_HIR_ID,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah, I guess that works 👍

we should somehow adjust the error message to say that this lint is always attached to the whole crate and can't be disabled on a per function basis

span.unwrap_or(DUMMY_SP),
rustc_errors::DiagDecorator(|diag| {
diag.primary_message(format!(
"reached the recursion limit {} when resolving trait bounds",
self.recursion_limit()
));
diag.help(format!(
"consider increasing it by adding a `#![recursion_limit = \"{}\"]`",
self.recursion_limit() * 2
));
}),
)
}

fn item_name(self, id: DefId) -> Symbol {
Expand Down
84 changes: 77 additions & 7 deletions compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,40 @@ where
return Ok(res);
}

let result = EvalCtxt::enter_root(self, self.cx().recursion_limit(), span, |ecx| {
// Fast paths handled above
ecx.evaluate_goal_no_fast_paths(GoalSource::Misc, goal)
});
let eval_with_recursion_limit = |limit| {
EvalCtxt::enter_root(self, limit, span, |ecx| {
ecx.evaluate_goal_no_fast_paths(GoalSource::Misc, goal)
})
};
let is_overflow_and_has_no_stalled_infers = |eval_result: &Result<GoalEvaluation<I>, _>| {
let predicate = match eval_result {
Err(_) => return false,
Ok(goal_evaluation) if !goal_evaluation.certainty.is_overflow() => return false,
Ok(goal_evaluation) => goal_evaluation.goal.predicate,
};

let has_no_stalled_infers = match predicate.kind().skip_binder() {
ty::PredicateKind::Clause(ty::ClauseKind::Projection(projection)) => {
!projection.projection_term.has_non_region_infer()
}
_ => !predicate.has_non_region_infer(),
};
has_no_stalled_infers
};

// The old solver doesn't check depth requirement when looking up cache
// while the next solver does so. Thus the next solver is more prone to
// overflow. We emit a FCW for this.
let mut result = eval_with_recursion_limit(self.cx().recursion_limit());
if is_overflow_and_has_no_stalled_infers(&result) {
let new_result = eval_with_recursion_limit(self.cx().recursion_limit() * 2);
if let Ok(goal_evaluation) = &new_result
&& goal_evaluation.certainty.is_yes()
{
self.cx().emit_next_solver_overflow_fcw(Some(span));
result = new_result;
}
}
Comment on lines +249 to +282

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move this into a separate functions, e.g. is_overflow_and_has_no_stalled_infer can probably be shared between the call sites

can you do the reevaluation in a commit_if_ok so that we don't apply the inference etc constraints of the reevaluation while ignoring its result?


match result {
Ok(i) => Ok(i),
Expand Down Expand Up @@ -302,7 +332,45 @@ where
goal: Goal<I, I::Predicate>,
span: I::Span,
) -> (Result<NestedNormalizationGoals<I>, NoSolution>, inspect::GoalEvaluation<I>) {
evaluate_root_goal_for_proof_tree(self, goal, span)
let is_overflow_and_has_no_stalled_infers =
|goal_evaluation: &inspect::GoalEvaluation<I>| {
match goal_evaluation.result {
Err(_) => return false,
Ok(response) if !response.value.certainty.is_overflow() => return false,
Ok(_) => {}
}

let predicate: I::Predicate = goal_evaluation.uncanonicalized_goal.predicate;
let has_no_stalled_infers = match predicate.kind().skip_binder() {
ty::PredicateKind::Clause(ty::ClauseKind::Projection(projection)) => {
!projection.projection_term.has_non_region_infer()
}
_ => !predicate.has_non_region_infer(),
};
has_no_stalled_infers
};

// The old solver doesn't check depth requirement when looking up cache
// while the next solver does so. Thus the next solver is more prone to
// overflow. We emit a FCW for this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

link to an issue/the lint track issue/somewhere here

let (mut result, mut goal_evaluation) =
evaluate_root_goal_for_proof_tree(self, goal, span, self.cx().recursion_limit());
if is_overflow_and_has_no_stalled_infers(&goal_evaluation) {
let (new_result, new_goal_evaluation) = evaluate_root_goal_for_proof_tree(
self,
goal,
span,
self.cx().recursion_limit() * 2,
);
if let Ok(response) = &new_goal_evaluation.result
&& response.value.certainty.is_yes()
{
self.cx().emit_next_solver_overflow_fcw(Some(span));
result = new_result;
goal_evaluation = new_goal_evaluation;
}
}
(result, goal_evaluation)
}
}

Expand Down Expand Up @@ -1701,11 +1769,12 @@ pub fn evaluate_root_goal_for_proof_tree_raw_provider<
>(
cx: I,
canonical_goal: CanonicalInput<I>,
root_depth: usize,
) -> (QueryResult<I>, I::Probe) {
let mut inspect = inspect::ProofTreeBuilder::new();
let (canonical_result, accessed_opaques) = SearchGraph::<D>::evaluate_root_goal_for_proof_tree(
cx,
cx.recursion_limit(),
root_depth,
canonical_goal,
&mut inspect,
);
Expand All @@ -1723,6 +1792,7 @@ pub(super) fn evaluate_root_goal_for_proof_tree<D: SolverDelegate<Interner = I>,
delegate: &D,
goal: Goal<I, I::Predicate>,
origin_span: I::Span,
root_depth: usize,
) -> (Result<NestedNormalizationGoals<I>, NoSolution>, inspect::GoalEvaluation<I>) {
let opaque_types = delegate.clone_opaque_types_lookup_table();
let (goal, opaque_types) = eager_resolve_vars(&**delegate, (goal, opaque_types));
Expand All @@ -1732,7 +1802,7 @@ pub(super) fn evaluate_root_goal_for_proof_tree<D: SolverDelegate<Interner = I>,
canonicalize_goal(delegate, goal, &opaque_types, typing_mode.into());

let (canonical_result, final_revision) =
delegate.cx().evaluate_root_goal_for_proof_tree_raw(canonical_goal);
delegate.cx().evaluate_root_goal_for_proof_tree_raw(canonical_goal, root_depth);

let proof_tree = inspect::GoalEvaluation {
uncanonicalized_goal: goal,
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_trait_selection/src/solve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ pub use select::InferCtxtSelectExt;

fn evaluate_root_goal_for_proof_tree_raw<'tcx>(
tcx: TyCtxt<'tcx>,
canonical_input: CanonicalInput<TyCtxt<'tcx>>,
key: (CanonicalInput<TyCtxt<'tcx>>, usize),
) -> (QueryResult<TyCtxt<'tcx>>, &'tcx inspect::Probe<TyCtxt<'tcx>>) {
evaluate_root_goal_for_proof_tree_raw_provider::<SolverDelegate<'tcx>, TyCtxt<'tcx>>(
tcx,
canonical_input,
tcx, key.0, key.1,
)
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_type_ir/src/interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,11 @@ pub trait Interner:
fn evaluate_root_goal_for_proof_tree_raw(
self,
canonical_goal: CanonicalInput<Self>,
root_depth: usize,
) -> (QueryResult<Self>, Self::Probe);

fn emit_next_solver_overflow_fcw(self, span: Option<Self::Span>);

fn item_name(self, item_index: Self::DefId) -> Self::Symbol;
}

Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_type_ir/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,20 @@ impl Certainty {
stalled_on_coroutines: StalledOnCoroutines::No,
})
}

pub fn is_yes(&self) -> bool {
match self {
Certainty::Yes => true,
Certainty::Maybe(_) => false,
}
}

pub fn is_overflow(&self) -> bool {
match self {
Certainty::Maybe(MaybeInfo { cause: MaybeCause::Overflow { .. }, .. }) => true,
_ => false,
}
}
}

/// Why we failed to evaluate a goal.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
//~| ERROR reached the recursion limit while instantiating `<VirtualWrapper<_, 1> as MyTrait>::virtualize`

//@ build-fail
//@ compile-flags: --diagnostic-width=100 -Zwrite-long-types-to-disk=yes
//@ compile-flags: --diagnostic-width=100 -Zwrite-long-types-to-disk=yes -Awarnings

// Regression test for #114484: This used to ICE during monomorphization, because we treated
// `<VirtualWrapper<...> as Pointee>::Metadata` as a rigid projection after reaching the recursion
Expand Down
24 changes: 24 additions & 0 deletions tests/ui/traits/next-solver/overflow/fcw-on-auto-trait.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
warning: reached the recursion limit 8 when resolving trait bounds
--> $DIR/fcw-on-auto-trait.rs:22:5
|
LL | require_sync::<Foo<Foo<Foo<Foo<Foo<Foo<()>>>>>>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider increasing it by adding a `#![recursion_limit = "16"]`
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #159228 <https://github.com/rust-lang/rust/issues/159228>
= note: `#[warn(next_trait_solver_overflow)]` (part of `#[warn(future_incompatible)]`) on by default

warning: reached the recursion limit 8 when resolving trait bounds
--> $DIR/fcw-on-auto-trait.rs:22:5
|
LL | require_sync::<Foo<Foo<Foo<Foo<Foo<Foo<()>>>>>>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider increasing it by adding a `#![recursion_limit = "16"]`
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #159228 <https://github.com/rust-lang/rust/issues/159228>
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

warning: 2 warnings emitted

27 changes: 27 additions & 0 deletions tests/ui/traits/next-solver/overflow/fcw-on-auto-trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//@ revisions: old next
//@[next] compile-flags: -Znext-solver
//@ check-pass

// The old solver doesn't verify depth when looking up cache.
// To avoid breakage, we evaluate with higher recursion limit in the next solver
// and emit an FCW for this.
// See the `NEXT_TRAIT_SOLVER_OVERFLOW` FCW.

#![recursion_limit = "8"]

// The field order matters 😂
#[allow(dead_code)]
struct Foo<T> {
t: T,
opt_t: Option<T>,
}

fn require_sync<T: Sync>() {}

fn main() {
require_sync::<Foo<Foo<Foo<Foo<Foo<Foo<()>>>>>>>();
//[next]~^ WARN: reached the recursion limit 8 when resolving trait bounds [next_trait_solver_overflow]
//[next]~| WARN: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
//[next]~| WARN: reached the recursion limit 8 when resolving trait bounds [next_trait_solver_overflow]
//[next]~| WARN: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
}
Loading
Loading