From 784c7a6cadf218b518734c4f21690334401ff83a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 10 Feb 2022 22:45:58 -0800 Subject: [PATCH 1/2] only mark projection as ambiguous if GAT substs are constrained --- .../rustc_infer/src/infer/type_variable.rs | 2 +- .../src/traits/project.rs | 57 +++++++++---------- .../src/traits/select/mod.rs | 27 ++++++++- .../generic-associated-types/issue-74824.rs | 1 - .../issue-74824.stderr | 11 +--- .../generic-associated-types/issue-93874.rs | 35 ++++++++++++ 6 files changed, 89 insertions(+), 44 deletions(-) create mode 100644 src/test/ui/generic-associated-types/issue-93874.rs diff --git a/compiler/rustc_infer/src/infer/type_variable.rs b/compiler/rustc_infer/src/infer/type_variable.rs index 82970f214fa66..6a968005f3eaa 100644 --- a/compiler/rustc_infer/src/infer/type_variable.rs +++ b/compiler/rustc_infer/src/infer/type_variable.rs @@ -259,7 +259,7 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { let index = self.values().push(TypeVariableData { origin }); assert_eq!(eq_key.vid.as_u32(), index as u32); - debug!("new_var(index={:?}, universe={:?}, origin={:?}", eq_key.vid, universe, origin,); + debug!("new_var(index={:?}, universe={:?}, origin={:?})", eq_key.vid, universe, origin); eq_key.vid } diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 36cc14610cb4b..a8d15d98eea57 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -1073,16 +1073,6 @@ fn project<'cx, 'tcx>( return Ok(Projected::Progress(Progress::error(selcx.tcx()))); } - // If the obligation contains any inference types or consts in associated - // type substs, then we don't assemble any candidates. - // This isn't really correct, but otherwise we can end up in a case where - // we constrain inference variables by selecting a single predicate, when - // we need to stay general. See issue #91762. - let (_, predicate_own_substs) = obligation.predicate.trait_ref_and_own_substs(selcx.tcx()); - if predicate_own_substs.iter().any(|g| g.has_infer_types_or_consts()) { - return Err(ProjectionError::TooManyCandidates); - } - let mut candidates = ProjectionCandidateSet::None; // Make sure that the following procedures are kept in order. ParamEnv @@ -1180,7 +1170,7 @@ fn assemble_candidates_from_trait_def<'cx, 'tcx>( ProjectionCandidate::TraitDef, bounds.iter(), true, - ) + ); } /// In the case of a trait object like @@ -1245,27 +1235,34 @@ fn assemble_candidates_from_predicates<'cx, 'tcx>( let bound_predicate = predicate.kind(); if let ty::PredicateKind::Projection(data) = predicate.kind().skip_binder() { let data = bound_predicate.rebind(data); - let same_def_id = data.projection_def_id() == obligation.predicate.item_def_id; - - let is_match = same_def_id - && infcx.probe(|_| { - selcx.match_projection_projections( - obligation, - data, - potentially_unnormalized_candidates, - ) - }); + if data.projection_def_id() != obligation.predicate.item_def_id { + continue; + } - if is_match { - candidate_set.push_candidate(ctor(data)); + let is_match = infcx.probe(|_| { + selcx.match_projection_projections( + obligation, + data, + potentially_unnormalized_candidates, + ) + }); - if potentially_unnormalized_candidates - && !obligation.predicate.has_infer_types_or_consts() - { - // HACK: Pick the first trait def candidate for a fully - // inferred predicate. This is to allow duplicates that - // differ only in normalization. - return; + match is_match { + Some(true) => { + candidate_set.push_candidate(ctor(data)); + + if potentially_unnormalized_candidates + && !obligation.predicate.has_infer_types_or_consts() + { + // HACK: Pick the first trait def candidate for a fully + // inferred predicate. This is to allow duplicates that + // differ only in normalization. + return; + } + } + Some(false) => {} + None => { + candidate_set.mark_ambiguous(); } } } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index a183a20a2fed0..4ca60a65485c5 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -1508,12 +1508,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { }) } + /// Return Some(true) if the obligation's predicate type applies to the env_predicate, and + /// Some(false) if it does not. Returns None in the case that the projection type is a GAT, + /// and applying this env_predicate constrains any of the obligation's GAT substitutions. pub(super) fn match_projection_projections( &mut self, obligation: &ProjectionTyObligation<'tcx>, env_predicate: PolyProjectionPredicate<'tcx>, potentially_unnormalized_candidates: bool, - ) -> bool { + ) -> Option { let mut nested_obligations = Vec::new(); let (infer_predicate, _) = self.infcx.replace_bound_vars_with_fresh_vars( obligation.cause.span, @@ -1535,7 +1538,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { infer_predicate.projection_ty }; - self.infcx + let is_match = self + .infcx .at(&obligation.cause, obligation.param_env) .define_opaque_types(false) .sup(obligation.predicate, infer_projection) @@ -1545,7 +1549,24 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { nested_obligations.into_iter().chain(obligations), ) .map_or(false, |res| res.may_apply()) - }) + }); + + if is_match { + let generics = self.tcx().generics_of(obligation.predicate.item_def_id); + if !generics.params.is_empty() { + // If any of the obligation's predicate substs shallow-resolve to + // something new, that means that we must have newly inferred something + // about the GAT. We should give up with ambiguity in that case. + if obligation.predicate.substs[generics.parent_count..] + .iter() + .any(|&p| p.has_infer_types_or_consts() && self.infcx.shallow_resolve(p) != p) + { + return None; + } + } + } + + Some(is_match) } /////////////////////////////////////////////////////////////////////////// diff --git a/src/test/ui/generic-associated-types/issue-74824.rs b/src/test/ui/generic-associated-types/issue-74824.rs index 01f99fa448749..1bbf7aac5cdab 100644 --- a/src/test/ui/generic-associated-types/issue-74824.rs +++ b/src/test/ui/generic-associated-types/issue-74824.rs @@ -17,7 +17,6 @@ impl UnsafeCopy for T {} fn main() { let b = Box::new(42usize); let copy = <()>::copy(&b); - //~^ type annotations needed let raw_b = Box::deref(&b) as *const _; let raw_copy = Box::deref(©) as *const _; diff --git a/src/test/ui/generic-associated-types/issue-74824.stderr b/src/test/ui/generic-associated-types/issue-74824.stderr index e7ebf5964ba41..8517eb9fa2102 100644 --- a/src/test/ui/generic-associated-types/issue-74824.stderr +++ b/src/test/ui/generic-associated-types/issue-74824.stderr @@ -27,13 +27,6 @@ help: consider restricting type parameter `T` LL | type Copy: Copy = Box; | +++++++++++++++++++ -error[E0282]: type annotations needed - --> $DIR/issue-74824.rs:19:16 - | -LL | let copy = <()>::copy(&b); - | ^^^^^^^^^^ cannot infer type for type parameter `T` declared on the associated function `copy` - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors -Some errors have detailed explanations: E0277, E0282. -For more information about an error, try `rustc --explain E0277`. +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/generic-associated-types/issue-93874.rs b/src/test/ui/generic-associated-types/issue-93874.rs new file mode 100644 index 0000000000000..f403d75167d8a --- /dev/null +++ b/src/test/ui/generic-associated-types/issue-93874.rs @@ -0,0 +1,35 @@ +// check-pass + +#![feature(generic_associated_types)] + +pub trait Build { + type Output; + fn build(self, input: O) -> Self::Output; +} + +pub struct IdentityBuild; +impl Build for IdentityBuild { + type Output = O; + fn build(self, input: O) -> Self::Output { + input + } +} + +fn a() { + let _x: u8 = IdentityBuild.build(10); +} + +fn b() { + let _x: Vec = IdentityBuild.build(Vec::new()); +} + +fn c() { + let mut f = IdentityBuild.build(|| ()); + (f)(); +} + +pub fn main() { + a(); + b(); + c(); +} From 879e4f8131b71050b00407befd6f1669389fe9ed Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 12 Feb 2022 13:30:30 -0800 Subject: [PATCH 2/2] use an enum in matches_projection_projection --- .../src/traits/project.rs | 7 ++-- .../src/traits/select/mod.rs | 37 ++++++++++++------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index a8d15d98eea57..584e412b8a228 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -19,6 +19,7 @@ use super::{Normalized, NormalizedTy, ProjectionCacheEntry, ProjectionCacheKey}; use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use crate::infer::{InferCtxt, InferOk, LateBoundRegionConversionTime}; use crate::traits::error_reporting::InferCtxtExt as _; +use crate::traits::select::ProjectionMatchesProjection; use rustc_data_structures::sso::SsoHashSet; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::ErrorReported; @@ -1248,7 +1249,7 @@ fn assemble_candidates_from_predicates<'cx, 'tcx>( }); match is_match { - Some(true) => { + ProjectionMatchesProjection::Yes => { candidate_set.push_candidate(ctor(data)); if potentially_unnormalized_candidates @@ -1260,10 +1261,10 @@ fn assemble_candidates_from_predicates<'cx, 'tcx>( return; } } - Some(false) => {} - None => { + ProjectionMatchesProjection::Ambiguous => { candidate_set.mark_ambiguous(); } + ProjectionMatchesProjection::No => {} } } } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 4ca60a65485c5..2f85417a5b618 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -1508,15 +1508,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { }) } - /// Return Some(true) if the obligation's predicate type applies to the env_predicate, and - /// Some(false) if it does not. Returns None in the case that the projection type is a GAT, + /// Return `Yes` if the obligation's predicate type applies to the env_predicate, and + /// `No` if it does not. Return `Ambiguous` in the case that the projection type is a GAT, /// and applying this env_predicate constrains any of the obligation's GAT substitutions. + /// + /// This behavior is a somewhat of a hack to prevent overconstraining inference variables + /// in cases like #91762. pub(super) fn match_projection_projections( &mut self, obligation: &ProjectionTyObligation<'tcx>, env_predicate: PolyProjectionPredicate<'tcx>, potentially_unnormalized_candidates: bool, - ) -> Option { + ) -> ProjectionMatchesProjection { let mut nested_obligations = Vec::new(); let (infer_predicate, _) = self.infcx.replace_bound_vars_with_fresh_vars( obligation.cause.span, @@ -1553,20 +1556,22 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { if is_match { let generics = self.tcx().generics_of(obligation.predicate.item_def_id); - if !generics.params.is_empty() { - // If any of the obligation's predicate substs shallow-resolve to - // something new, that means that we must have newly inferred something - // about the GAT. We should give up with ambiguity in that case. - if obligation.predicate.substs[generics.parent_count..] + // FIXME(generic-associated-types): Addresses aggressive inference in #92917. + // If this type is a GAT, and of the GAT substs resolve to something new, + // that means that we must have newly inferred something about the GAT. + // We should give up in that case. + if !generics.params.is_empty() + && obligation.predicate.substs[generics.parent_count..] .iter() .any(|&p| p.has_infer_types_or_consts() && self.infcx.shallow_resolve(p) != p) - { - return None; - } + { + ProjectionMatchesProjection::Ambiguous + } else { + ProjectionMatchesProjection::Yes } + } else { + ProjectionMatchesProjection::No } - - Some(is_match) } /////////////////////////////////////////////////////////////////////////// @@ -2766,3 +2771,9 @@ impl<'o, 'tcx> fmt::Debug for TraitObligationStack<'o, 'tcx> { write!(f, "TraitObligationStack({:?})", self.obligation) } } + +pub enum ProjectionMatchesProjection { + Yes, + Ambiguous, + No, +}