From b335c2d49f1698ada99e3622d3cd482c27c9fc9b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 11 Apr 2023 21:15:39 +0000 Subject: [PATCH 1/2] Assemble Unpin candidates specially for generators in new solver --- .../src/solve/trait_goals.rs | 19 ++++++++++++++++++- tests/ui/generator/non-static-is-unpin.rs | 2 ++ ...stderr => static-not-unpin.current.stderr} | 8 ++++---- .../ui/generator/static-not-unpin.next.stderr | 19 +++++++++++++++++++ tests/ui/generator/static-not-unpin.rs | 3 +++ 5 files changed, 46 insertions(+), 5 deletions(-) rename tests/ui/generator/{static-not-unpin.stderr => static-not-unpin.current.stderr} (71%) create mode 100644 tests/ui/generator/static-not-unpin.next.stderr diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs index 716d5acb324a2..73c599e181405 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs @@ -3,7 +3,7 @@ use super::assembly::{self, structural_traits}; use super::{EvalCtxt, SolverMode}; use rustc_hir::def_id::DefId; -use rustc_hir::LangItem; +use rustc_hir::{LangItem, Movability}; use rustc_infer::traits::query::NoSolution; use rustc_infer::traits::util::supertraits; use rustc_middle::traits::solve::{CanonicalResponse, Certainty, Goal, QueryResult}; @@ -168,6 +168,23 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { ty::Infer(_) | ty::Bound(_, _) => bug!("unexpected type `{self_ty}`"), + // Generators have one special built-in candidate, `Unpin`, which + // takes precedence over the structural auto trait candidate being + // assembled. + ty::Generator(_, _, movability) + if Some(goal.predicate.def_id()) == ecx.tcx().lang_items().unpin_trait() => + { + match movability { + Movability::Static => { + return Err(NoSolution); + } + Movability::Movable => { + return ecx + .evaluate_added_goals_and_make_canonical_response(Certainty::Yes); + } + } + } + // For rigid types, we only register a builtin auto implementation // if there is no implementation that could ever apply to the self // type. diff --git a/tests/ui/generator/non-static-is-unpin.rs b/tests/ui/generator/non-static-is-unpin.rs index 96d0a8e283372..17e23f5bcd2fa 100644 --- a/tests/ui/generator/non-static-is-unpin.rs +++ b/tests/ui/generator/non-static-is-unpin.rs @@ -1,3 +1,5 @@ +// revisions: current next +//[next] compile-flags: -Ztrait-solver=next // run-pass #![feature(generators, generator_trait)] diff --git a/tests/ui/generator/static-not-unpin.stderr b/tests/ui/generator/static-not-unpin.current.stderr similarity index 71% rename from tests/ui/generator/static-not-unpin.stderr rename to tests/ui/generator/static-not-unpin.current.stderr index 7376116b3380a..ecd8ca60c6f2c 100644 --- a/tests/ui/generator/static-not-unpin.stderr +++ b/tests/ui/generator/static-not-unpin.current.stderr @@ -1,15 +1,15 @@ -error[E0277]: `[static generator@$DIR/static-not-unpin.rs:11:25: 11:34]` cannot be unpinned - --> $DIR/static-not-unpin.rs:14:18 +error[E0277]: `[static generator@$DIR/static-not-unpin.rs:14:25: 14:34]` cannot be unpinned + --> $DIR/static-not-unpin.rs:17:18 | LL | assert_unpin(generator); - | ------------ ^^^^^^^^^ the trait `Unpin` is not implemented for `[static generator@$DIR/static-not-unpin.rs:11:25: 11:34]` + | ------------ ^^^^^^^^^ the trait `Unpin` is not implemented for `[static generator@$DIR/static-not-unpin.rs:14:25: 14:34]` | | | required by a bound introduced by this call | = note: consider using the `pin!` macro consider using `Box::pin` if you need to access the pinned value outside of the current scope note: required by a bound in `assert_unpin` - --> $DIR/static-not-unpin.rs:7:20 + --> $DIR/static-not-unpin.rs:10:20 | LL | fn assert_unpin(_: T) { | ^^^^^ required by this bound in `assert_unpin` diff --git a/tests/ui/generator/static-not-unpin.next.stderr b/tests/ui/generator/static-not-unpin.next.stderr new file mode 100644 index 0000000000000..ecd8ca60c6f2c --- /dev/null +++ b/tests/ui/generator/static-not-unpin.next.stderr @@ -0,0 +1,19 @@ +error[E0277]: `[static generator@$DIR/static-not-unpin.rs:14:25: 14:34]` cannot be unpinned + --> $DIR/static-not-unpin.rs:17:18 + | +LL | assert_unpin(generator); + | ------------ ^^^^^^^^^ the trait `Unpin` is not implemented for `[static generator@$DIR/static-not-unpin.rs:14:25: 14:34]` + | | + | required by a bound introduced by this call + | + = note: consider using the `pin!` macro + consider using `Box::pin` if you need to access the pinned value outside of the current scope +note: required by a bound in `assert_unpin` + --> $DIR/static-not-unpin.rs:10:20 + | +LL | fn assert_unpin(_: T) { + | ^^^^^ required by this bound in `assert_unpin` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/generator/static-not-unpin.rs b/tests/ui/generator/static-not-unpin.rs index cfcb94737be6f..30d3f29187096 100644 --- a/tests/ui/generator/static-not-unpin.rs +++ b/tests/ui/generator/static-not-unpin.rs @@ -1,3 +1,6 @@ +// revisions: current next +//[next] compile-flags: -Ztrait-solver=next + #![feature(generators)] // normalize-stderr-test "std::pin::Unpin" -> "std::marker::Unpin" From 319c790600c2f8bfccf112da216e64566a61e5b6 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 12 Apr 2023 18:56:19 +0000 Subject: [PATCH 2/2] Move auto trait built-in candidate disqualification to a separate method --- .../src/solve/assembly/mod.rs | 8 + .../src/solve/trait_goals.rs | 170 ++++++++++-------- 2 files changed, 101 insertions(+), 77 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs index 7f0ceb6646ceb..10d817f75ac77 100644 --- a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs @@ -348,6 +348,14 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { ) { let lang_items = self.tcx().lang_items(); let trait_def_id = goal.predicate.trait_def_id(self.tcx()); + + // N.B. When assembling built-in candidates for lang items that are also + // `auto` traits, then the auto trait candidate that is assembled in + // `consider_auto_trait_candidate` MUST be disqualified to remain sound. + // + // Instead of adding the logic here, it's a better idea to add it in + // `EvalCtxt::disqualify_auto_trait_candidate_due_to_possible_impl` in + // `solve::trait_goals` instead. let result = if self.tcx().trait_is_auto(trait_def_id) { G::consider_auto_trait_candidate(self, goal) } else if self.tcx().trait_is_alias(trait_def_id) { diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs index 73c599e181405..abd11a15ac23a 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs @@ -147,83 +147,8 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { ecx: &mut EvalCtxt<'_, 'tcx>, goal: Goal<'tcx, Self>, ) -> QueryResult<'tcx> { - let self_ty = goal.predicate.self_ty(); - match *self_ty.kind() { - // Stall int and float vars until they are resolved to a concrete - // numerical type. That's because the check for impls below treats - // int vars as matching any impl. Even if we filtered such impls, - // we probably don't want to treat an `impl !AutoTrait for i32` as - // disqualifying the built-in auto impl for `i64: AutoTrait` either. - ty::Infer(ty::IntVar(_) | ty::FloatVar(_)) => { - return ecx.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS); - } - - // These types cannot be structurally decomposed into constitutent - // types, and therefore have no builtin impl. - ty::Dynamic(..) - | ty::Param(..) - | ty::Foreign(..) - | ty::Alias(ty::Projection, ..) - | ty::Placeholder(..) => return Err(NoSolution), - - ty::Infer(_) | ty::Bound(_, _) => bug!("unexpected type `{self_ty}`"), - - // Generators have one special built-in candidate, `Unpin`, which - // takes precedence over the structural auto trait candidate being - // assembled. - ty::Generator(_, _, movability) - if Some(goal.predicate.def_id()) == ecx.tcx().lang_items().unpin_trait() => - { - match movability { - Movability::Static => { - return Err(NoSolution); - } - Movability::Movable => { - return ecx - .evaluate_added_goals_and_make_canonical_response(Certainty::Yes); - } - } - } - - // For rigid types, we only register a builtin auto implementation - // if there is no implementation that could ever apply to the self - // type. - // - // This differs from the current stable behavior and fixes #84857. - // Due to breakage found via crater, we currently instead lint - // patterns which can be used to exploit this unsoundness on stable, - // see #93367 for more details. - ty::Bool - | ty::Char - | ty::Int(_) - | ty::Uint(_) - | ty::Float(_) - | ty::Str - | ty::Array(_, _) - | ty::Slice(_) - | ty::RawPtr(_) - | ty::Ref(_, _, _) - | ty::FnDef(_, _) - | ty::FnPtr(_) - | ty::Closure(_, _) - | ty::Generator(_, _, _) - | ty::GeneratorWitness(_) - | ty::GeneratorWitnessMIR(_, _) - | ty::Never - | ty::Tuple(_) - | ty::Error(_) - | ty::Adt(_, _) - | ty::Alias(ty::Opaque, _) => { - if let Some(def_id) = ecx.tcx().find_map_relevant_impl( - goal.predicate.def_id(), - goal.predicate.self_ty(), - TreatProjections::NextSolverLookup, - Some, - ) { - debug!(?def_id, ?goal, "disqualified auto-trait implementation"); - return Err(NoSolution); - } - } + if let Some(result) = ecx.disqualify_auto_trait_candidate_due_to_possible_impl(goal) { + return result; } ecx.probe_and_evaluate_goal_for_constituent_tys( @@ -647,6 +572,97 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { } impl<'tcx> EvalCtxt<'_, 'tcx> { + // Return `Some` if there is an impl (built-in or user provided) that may + // hold for the self type of the goal, which for coherence and soundness + // purposes must disqualify the built-in auto impl assembled by considering + // the type's constituent types. + fn disqualify_auto_trait_candidate_due_to_possible_impl( + &mut self, + goal: Goal<'tcx, TraitPredicate<'tcx>>, + ) -> Option> { + let self_ty = goal.predicate.self_ty(); + match *self_ty.kind() { + // Stall int and float vars until they are resolved to a concrete + // numerical type. That's because the check for impls below treats + // int vars as matching any impl. Even if we filtered such impls, + // we probably don't want to treat an `impl !AutoTrait for i32` as + // disqualifying the built-in auto impl for `i64: AutoTrait` either. + ty::Infer(ty::IntVar(_) | ty::FloatVar(_)) => { + Some(self.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)) + } + + // These types cannot be structurally decomposed into constitutent + // types, and therefore have no built-in auto impl. + ty::Dynamic(..) + | ty::Param(..) + | ty::Foreign(..) + | ty::Alias(ty::Projection, ..) + | ty::Placeholder(..) => Some(Err(NoSolution)), + + ty::Infer(_) | ty::Bound(_, _) => bug!("unexpected type `{self_ty}`"), + + // Generators have one special built-in candidate, `Unpin`, which + // takes precedence over the structural auto trait candidate being + // assembled. + ty::Generator(_, _, movability) + if Some(goal.predicate.def_id()) == self.tcx().lang_items().unpin_trait() => + { + match movability { + Movability::Static => Some(Err(NoSolution)), + Movability::Movable => { + Some(self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)) + } + } + } + + // For rigid types, any possible implementation that could apply to + // the type (even if after unification and processing nested goals + // it does not hold) will disqualify the built-in auto impl. + // + // This differs from the current stable behavior and fixes #84857. + // Due to breakage found via crater, we currently instead lint + // patterns which can be used to exploit this unsoundness on stable, + // see #93367 for more details. + ty::Bool + | ty::Char + | ty::Int(_) + | ty::Uint(_) + | ty::Float(_) + | ty::Str + | ty::Array(_, _) + | ty::Slice(_) + | ty::RawPtr(_) + | ty::Ref(_, _, _) + | ty::FnDef(_, _) + | ty::FnPtr(_) + | ty::Closure(_, _) + | ty::Generator(_, _, _) + | ty::GeneratorWitness(_) + | ty::GeneratorWitnessMIR(_, _) + | ty::Never + | ty::Tuple(_) + | ty::Adt(_, _) + // FIXME: Handling opaques here is kinda sus. Especially because we + // simplify them to PlaceholderSimplifiedType. + | ty::Alias(ty::Opaque, _) => { + if let Some(def_id) = self.tcx().find_map_relevant_impl( + goal.predicate.def_id(), + goal.predicate.self_ty(), + TreatProjections::NextSolverLookup, + Some, + ) { + debug!(?def_id, ?goal, "disqualified auto-trait implementation"); + // No need to actually consider the candidate here, + // since we do that in `consider_impl_candidate`. + return Some(Err(NoSolution)); + } else { + None + } + } + ty::Error(_) => None, + } + } + /// Convenience function for traits that are structural, i.e. that only /// have nested subgoals that only change the self type. Unlike other /// evaluate-like helpers, this does a probe, so it doesn't need to be