From 0d71860368094f21b2865c55685fe76a8e4a5874 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 8 Jan 2024 15:29:44 +0100 Subject: [PATCH] bye bye `assemble_candidates_via_self_ty` --- .../src/solve/assembly/mod.rs | 142 ++++-------------- .../rustc_trait_selection/src/solve/mod.rs | 8 +- .../traits/next-solver/alias-bound-unsound.rs | 2 +- .../next-solver/alias-bound-unsound.stderr | 14 +- .../ambig-projection-self-is-ambig.rs | 19 +++ ...e_of-tait-in-defining-scope.is_send.stderr | 16 ++ ..._of-tait-in-defining-scope.not_send.stderr | 4 +- .../dont-type_of-tait-in-defining-scope.rs | 3 +- .../recursive-self-normalization-2.rs | 1 + .../recursive-self-normalization-2.stderr | 19 ++- .../overflow/recursive-self-normalization.rs | 1 + .../recursive-self-normalization.stderr | 19 ++- .../issue-76202-trait-impl-for-tait.rs | 22 +-- 13 files changed, 133 insertions(+), 137 deletions(-) create mode 100644 tests/ui/traits/next-solver/assembly/ambig-projection-self-is-ambig.rs create mode 100644 tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.is_send.stderr diff --git a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs index caf9470b4c6..47fc5f00755 100644 --- a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs @@ -271,12 +271,40 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { &mut self, goal: Goal<'tcx, G>, ) -> Vec> { - debug_assert_eq!(goal, self.resolve_vars_if_possible(goal)); - if let Some(ambig) = self.assemble_self_ty_infer_ambiguity_response(goal) { - return vec![ambig]; + let dummy_candidate = |this: &mut EvalCtxt<'_, 'tcx>, certainty| { + let source = CandidateSource::BuiltinImpl(BuiltinImplSource::Misc); + let result = this.evaluate_added_goals_and_make_canonical_response(certainty).unwrap(); + let mut dummy_probe = this.inspect.new_probe(); + dummy_probe.probe_kind(ProbeKind::TraitCandidate { source, result: Ok(result) }); + this.inspect.finish_probe(dummy_probe); + vec![Candidate { source, result }] + }; + + let Some(normalized_self_ty) = + self.try_normalize_ty(goal.param_env, goal.predicate.self_ty()) + else { + debug!("overflow while evaluating self type"); + return dummy_candidate(self, Certainty::OVERFLOW); + }; + + if normalized_self_ty.is_ty_var() { + debug!("self type has been normalized to infer"); + return dummy_candidate(self, Certainty::AMBIGUOUS); } - let mut candidates = self.assemble_candidates_via_self_ty(goal, 0); + let goal = + goal.with(self.tcx(), goal.predicate.with_self_ty(self.tcx(), normalized_self_ty)); + debug_assert_eq!(goal, self.resolve_vars_if_possible(goal)); + + let mut candidates = vec![]; + + self.assemble_non_blanket_impl_candidates(goal, &mut candidates); + + self.assemble_builtin_impl_candidates(goal, &mut candidates); + + self.assemble_alias_bound_candidates(goal, &mut candidates); + + self.assemble_object_bound_candidates(goal, &mut candidates); self.assemble_unsize_to_dyn_candidate(goal, &mut candidates); @@ -289,112 +317,6 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { candidates } - /// `?0: Trait` is ambiguous, because it may be satisfied via a builtin rule, - /// object bound, alias bound, etc. We are unable to determine this until we can at - /// least structurally resolve the type one layer. - /// - /// It would also require us to consider all impls of the trait, which is both pretty - /// bad for perf and would also constrain the self type if there is just a single impl. - fn assemble_self_ty_infer_ambiguity_response>( - &mut self, - goal: Goal<'tcx, G>, - ) -> Option> { - if goal.predicate.self_ty().is_ty_var() { - debug!("adding self_ty_infer_ambiguity_response"); - let source = CandidateSource::BuiltinImpl(BuiltinImplSource::Misc); - let result = self - .evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS) - .unwrap(); - let mut dummy_probe = self.inspect.new_probe(); - dummy_probe.probe_kind(ProbeKind::TraitCandidate { source, result: Ok(result) }); - self.inspect.finish_probe(dummy_probe); - Some(Candidate { source, result }) - } else { - None - } - } - - /// Assemble candidates which apply to the self type. This only looks at candidate which - /// apply to the specific self type and ignores all others. - /// - /// Returns `None` if the self type is still ambiguous. - fn assemble_candidates_via_self_ty>( - &mut self, - goal: Goal<'tcx, G>, - num_steps: usize, - ) -> Vec> { - debug_assert_eq!(goal, self.resolve_vars_if_possible(goal)); - if let Some(ambig) = self.assemble_self_ty_infer_ambiguity_response(goal) { - return vec![ambig]; - } - - let mut candidates = Vec::new(); - - self.assemble_non_blanket_impl_candidates(goal, &mut candidates); - - self.assemble_builtin_impl_candidates(goal, &mut candidates); - - self.assemble_alias_bound_candidates(goal, &mut candidates); - - self.assemble_object_bound_candidates(goal, &mut candidates); - - self.assemble_candidates_after_normalizing_self_ty(goal, &mut candidates, num_steps); - candidates - } - - /// If the self type of a goal is an alias we first try to normalize the self type - /// and compute the candidates for the normalized self type in case that succeeds. - /// - /// These candidates are used in addition to the ones with the alias as a self type. - /// We do this to simplify both builtin candidates and for better performance. - /// - /// We generate the builtin candidates on the fly by looking at the self type, e.g. - /// add `FnPtr` candidates if the self type is a function pointer. Handling builtin - /// candidates while the self type is still an alias seems difficult. This is similar - /// to `try_structurally_resolve_type` during hir typeck (FIXME once implemented). - /// - /// Looking at all impls for some trait goal is prohibitively expensive. We therefore - /// only look at implementations with a matching self type. Because of this function, - /// we can avoid looking at all existing impls if the self type is an alias. - #[instrument(level = "debug", skip_all)] - fn assemble_candidates_after_normalizing_self_ty>( - &mut self, - goal: Goal<'tcx, G>, - candidates: &mut Vec>, - num_steps: usize, - ) { - let tcx = self.tcx(); - let &ty::Alias(_, alias) = goal.predicate.self_ty().kind() else { return }; - - candidates.extend(self.probe(|_| ProbeKind::NormalizedSelfTyAssembly).enter(|ecx| { - if tcx.recursion_limit().value_within_limit(num_steps) { - let normalized_ty = ecx.next_ty_infer(); - let normalizes_to_goal = - goal.with(tcx, ty::NormalizesTo { alias, term: normalized_ty.into() }); - ecx.add_goal(GoalSource::Misc, normalizes_to_goal); - if let Err(NoSolution) = ecx.try_evaluate_added_goals() { - debug!("self type normalization failed"); - return vec![]; - } - let normalized_ty = ecx.resolve_vars_if_possible(normalized_ty); - debug!(?normalized_ty, "self type normalized"); - // NOTE: Alternatively we could call `evaluate_goal` here and only - // have a `Normalized` candidate. This doesn't work as long as we - // use `CandidateSource` in winnowing. - let goal = goal.with(tcx, goal.predicate.with_self_ty(tcx, normalized_ty)); - ecx.assemble_candidates_via_self_ty(goal, num_steps + 1) - } else { - match ecx.evaluate_added_goals_and_make_canonical_response(Certainty::OVERFLOW) { - Ok(result) => vec![Candidate { - source: CandidateSource::BuiltinImpl(BuiltinImplSource::Misc), - result, - }], - Err(NoSolution) => vec![], - } - } - })); - } - #[instrument(level = "debug", skip_all)] fn assemble_non_blanket_impl_candidates>( &mut self, diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs index 7c8f885a1f2..6984f0ba694 100644 --- a/compiler/rustc_trait_selection/src/solve/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/mod.rs @@ -288,11 +288,9 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { /// Normalize a type when it is structually matched on. /// - /// For self types this is generally already handled through - /// `assemble_candidates_after_normalizing_self_ty`, so anything happening - /// in [`EvalCtxt::assemble_candidates_via_self_ty`] does not have to normalize - /// the self type. It is required when structurally matching on any other - /// arguments of a trait goal, e.g. when assembling builtin unsize candidates. + /// In nearly all cases this function must be used before matching on a type. + /// Not doing so is likely to be incomplete and therefore unsound during + /// coherence. #[instrument(level = "debug", skip(self), ret)] fn try_normalize_ty( &mut self, diff --git a/tests/ui/traits/next-solver/alias-bound-unsound.rs b/tests/ui/traits/next-solver/alias-bound-unsound.rs index 8fddbd7ecdc..633f0e75572 100644 --- a/tests/ui/traits/next-solver/alias-bound-unsound.rs +++ b/tests/ui/traits/next-solver/alias-bound-unsound.rs @@ -16,7 +16,7 @@ trait Foo { impl Foo for () { type Item = String where String: Copy; - //~^ ERROR overflow evaluating the requirement `<() as Foo>::Item: Copy` + //~^ ERROR overflow evaluating the requirement `String: Copy` } fn main() { diff --git a/tests/ui/traits/next-solver/alias-bound-unsound.stderr b/tests/ui/traits/next-solver/alias-bound-unsound.stderr index 874644317eb..d5f7e975b13 100644 --- a/tests/ui/traits/next-solver/alias-bound-unsound.stderr +++ b/tests/ui/traits/next-solver/alias-bound-unsound.stderr @@ -1,15 +1,17 @@ -error[E0275]: overflow evaluating the requirement `<() as Foo>::Item: Copy` - --> $DIR/alias-bound-unsound.rs:18:17 +error[E0275]: overflow evaluating the requirement `String: Copy` + --> $DIR/alias-bound-unsound.rs:18:38 | LL | type Item = String where String: Copy; - | ^^^^^^ + | ^^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`alias_bound_unsound`) -note: required by a bound in `Foo::Item` - --> $DIR/alias-bound-unsound.rs:8:16 +note: the requirement `String: Copy` appears on the `impl`'s associated type `Item` but not on the corresponding trait's associated type + --> $DIR/alias-bound-unsound.rs:8:10 | +LL | trait Foo { + | --- in this trait LL | type Item: Copy - | ^^^^ required by this bound in `Foo::Item` + | ^^^^ this trait's associated type doesn't have the requirement `String: Copy` error[E0275]: overflow evaluating the requirement `String <: <() as Foo>::Item` --> $DIR/alias-bound-unsound.rs:24:31 diff --git a/tests/ui/traits/next-solver/assembly/ambig-projection-self-is-ambig.rs b/tests/ui/traits/next-solver/assembly/ambig-projection-self-is-ambig.rs new file mode 100644 index 00000000000..99a368a746f --- /dev/null +++ b/tests/ui/traits/next-solver/assembly/ambig-projection-self-is-ambig.rs @@ -0,0 +1,19 @@ +// check-pass +// compile-flags: -Znext-solver + +trait Reader: Default { + fn read_u8_array(&self) -> Result { + todo!() + } + + fn read_u8(&self) -> Result { + let a: [u8; 1] = self.read_u8_array::<_>()?; + // This results in a nested ` as Try>::Residual: Sized` goal. + // The self type normalizes to `?0`. We previously did not force that to be + // ambiguous but instead incompletely applied the `Self: Sized` candidate + // from the `ParamEnv`, resulting in a type error. + Ok(a[0]) + } +} + +fn main() {} diff --git a/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.is_send.stderr b/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.is_send.stderr new file mode 100644 index 00000000000..bafc4ba18a7 --- /dev/null +++ b/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.is_send.stderr @@ -0,0 +1,16 @@ +error[E0283]: type annotations needed: cannot satisfy `Foo: Send` + --> $DIR/dont-type_of-tait-in-defining-scope.rs:15:18 + | +LL | needs_send::(); + | ^^^ + | + = note: cannot satisfy `Foo: Send` +note: required by a bound in `needs_send` + --> $DIR/dont-type_of-tait-in-defining-scope.rs:12:18 + | +LL | fn needs_send() {} + | ^^^^ required by this bound in `needs_send` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0283`. diff --git a/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.not_send.stderr b/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.not_send.stderr index 076dab29d89..bafc4ba18a7 100644 --- a/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.not_send.stderr +++ b/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.not_send.stderr @@ -1,12 +1,12 @@ error[E0283]: type annotations needed: cannot satisfy `Foo: Send` - --> $DIR/dont-type_of-tait-in-defining-scope.rs:16:18 + --> $DIR/dont-type_of-tait-in-defining-scope.rs:15:18 | LL | needs_send::(); | ^^^ | = note: cannot satisfy `Foo: Send` note: required by a bound in `needs_send` - --> $DIR/dont-type_of-tait-in-defining-scope.rs:13:18 + --> $DIR/dont-type_of-tait-in-defining-scope.rs:12:18 | LL | fn needs_send() {} | ^^^^ required by this bound in `needs_send` diff --git a/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.rs b/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.rs index a1f38e69e53..ef0360248b5 100644 --- a/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.rs +++ b/tests/ui/traits/next-solver/dont-type_of-tait-in-defining-scope.rs @@ -1,6 +1,5 @@ // revisions: is_send not_send // compile-flags: -Znext-solver -//[is_send] check-pass #![feature(type_alias_impl_trait)] @@ -14,7 +13,7 @@ fn needs_send() {} fn test(_: Foo) { needs_send::(); - //[not_send]~^ ERROR type annotations needed: cannot satisfy `Foo: Send` + //~^ ERROR type annotations needed: cannot satisfy `Foo: Send` } fn defines(_: Foo) { diff --git a/tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.rs b/tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.rs index 932826519b7..983a0fec653 100644 --- a/tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.rs +++ b/tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.rs @@ -17,6 +17,7 @@ fn test::Assoc2> + Foo2::Assoc //~| ERROR overflow evaluating the requirement `::Assoc1 == _` //~| ERROR overflow evaluating the requirement `::Assoc1 == _` //~| ERROR overflow evaluating the requirement `::Assoc1 == _` + //~| ERROR overflow evaluating the requirement `::Assoc1: Sized` //~| ERROR overflow evaluating the requirement `::Assoc1: Bar` } diff --git a/tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.stderr b/tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.stderr index e4f1f9cf022..ed87404d573 100644 --- a/tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.stderr +++ b/tests/ui/traits/next-solver/overflow/recursive-self-normalization-2.stderr @@ -19,6 +19,23 @@ note: required by a bound in `needs_bar` LL | fn needs_bar() {} | ^^^ required by this bound in `needs_bar` +error[E0275]: overflow evaluating the requirement `::Assoc1: Sized` + --> $DIR/recursive-self-normalization-2.rs:15:17 + | +LL | needs_bar::(); + | ^^^^^^^^^ + | + = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization_2`) +note: required by a bound in `needs_bar` + --> $DIR/recursive-self-normalization-2.rs:12:14 + | +LL | fn needs_bar() {} + | ^ required by this bound in `needs_bar` +help: consider relaxing the implicit `Sized` restriction + | +LL | fn needs_bar() {} + | ++++++++ + error[E0275]: overflow evaluating the requirement `::Assoc1 == _` --> $DIR/recursive-self-normalization-2.rs:15:5 | @@ -45,6 +62,6 @@ LL | needs_bar::(); = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization_2`) = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0275`. diff --git a/tests/ui/traits/next-solver/overflow/recursive-self-normalization.rs b/tests/ui/traits/next-solver/overflow/recursive-self-normalization.rs index 32672c08c7e..40e2aa9e63f 100644 --- a/tests/ui/traits/next-solver/overflow/recursive-self-normalization.rs +++ b/tests/ui/traits/next-solver/overflow/recursive-self-normalization.rs @@ -13,6 +13,7 @@ fn test::Assoc>>() { //~| ERROR overflow evaluating the requirement `::Assoc == _` //~| ERROR overflow evaluating the requirement `::Assoc == _` //~| ERROR overflow evaluating the requirement `::Assoc == _` + //~| ERROR overflow evaluating the requirement `::Assoc: Sized` //~| ERROR overflow evaluating the requirement `::Assoc: Bar` } diff --git a/tests/ui/traits/next-solver/overflow/recursive-self-normalization.stderr b/tests/ui/traits/next-solver/overflow/recursive-self-normalization.stderr index da5c8bde568..e4ef2f60740 100644 --- a/tests/ui/traits/next-solver/overflow/recursive-self-normalization.stderr +++ b/tests/ui/traits/next-solver/overflow/recursive-self-normalization.stderr @@ -19,6 +19,23 @@ note: required by a bound in `needs_bar` LL | fn needs_bar() {} | ^^^ required by this bound in `needs_bar` +error[E0275]: overflow evaluating the requirement `::Assoc: Sized` + --> $DIR/recursive-self-normalization.rs:11:17 + | +LL | needs_bar::(); + | ^^^^^^^^ + | + = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization`) +note: required by a bound in `needs_bar` + --> $DIR/recursive-self-normalization.rs:8:14 + | +LL | fn needs_bar() {} + | ^ required by this bound in `needs_bar` +help: consider relaxing the implicit `Sized` restriction + | +LL | fn needs_bar() {} + | ++++++++ + error[E0275]: overflow evaluating the requirement `::Assoc == _` --> $DIR/recursive-self-normalization.rs:11:5 | @@ -45,6 +62,6 @@ LL | needs_bar::(); = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization`) = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0275`. diff --git a/tests/ui/type-alias-impl-trait/issue-76202-trait-impl-for-tait.rs b/tests/ui/type-alias-impl-trait/issue-76202-trait-impl-for-tait.rs index af1c18bbb59..b6906f68ded 100644 --- a/tests/ui/type-alias-impl-trait/issue-76202-trait-impl-for-tait.rs +++ b/tests/ui/type-alias-impl-trait/issue-76202-trait-impl-for-tait.rs @@ -4,20 +4,14 @@ // revisions: current next //[next] compile-flags: -Znext-solver // check-pass - #![feature(type_alias_impl_trait)] -trait Dummy {} -impl Dummy for () {} - -type F = impl Dummy; -fn f() -> F {} - trait Test { fn test(self); } -impl Test for F { + +impl Test for define::F { fn test(self) {} } @@ -27,7 +21,17 @@ impl Test for i32 { fn test(self) {} } +mod define { + use super::*; + + pub trait Dummy {} + impl Dummy for () {} + + pub type F = impl Dummy; + pub fn f() -> F {} +} + fn main() { - let x: F = f(); + let x = define::f(); x.test(); }