Skip to content

Commit

Permalink
Rollup merge of rust-lang#93892 - compiler-errors:issue-92917, r=jack…
Browse files Browse the repository at this point in the history
…h726,nikomatsakis

Only mark projection as ambiguous if GAT substs are constrained

A slightly more targeted version of rust-lang#92917, where we only give up with ambiguity if we infer something about the GATs substs when probing for a projection candidate.

fixes rust-lang#93874
also note (but like the previous PR, does not fix) rust-lang#91762

r? `@jackh726`
cc `@nikomatsakis` who reviewed rust-lang#92917
  • Loading branch information
matthiaskrgr authored Feb 18, 2022
2 parents cb35370 + 879e4f8 commit 1e2f63d
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 44 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/type_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,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
}
Expand Down
58 changes: 28 additions & 30 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1075,16 +1076,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
Expand Down Expand Up @@ -1182,7 +1173,7 @@ fn assemble_candidates_from_trait_def<'cx, 'tcx>(
ProjectionCandidate::TraitDef,
bounds.iter(),
true,
)
);
}

/// In the case of a trait object like
Expand Down Expand Up @@ -1247,28 +1238,35 @@ 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 {
ProjectionMatchesProjection::Yes => {
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;
}
}
ProjectionMatchesProjection::Ambiguous => {
candidate_set.mark_ambiguous();
}
ProjectionMatchesProjection::No => {}
}
}
}
Expand Down
38 changes: 35 additions & 3 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1494,12 +1494,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
})
}

/// 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,
) -> bool {
) -> ProjectionMatchesProjection {
let mut nested_obligations = Vec::new();
let (infer_predicate, _) = self.infcx.replace_bound_vars_with_fresh_vars(
obligation.cause.span,
Expand All @@ -1521,7 +1527,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
infer_predicate.projection_ty
};

self.infcx
let is_match = self
.infcx
.at(&obligation.cause, obligation.param_env)
.sup(obligation.predicate, infer_projection)
.map_or(false, |InferOk { obligations, value: () }| {
Expand All @@ -1530,7 +1537,26 @@ 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);
// 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)
{
ProjectionMatchesProjection::Ambiguous
} else {
ProjectionMatchesProjection::Yes
}
} else {
ProjectionMatchesProjection::No
}
}

///////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -2709,3 +2735,9 @@ impl<'o, 'tcx> fmt::Debug for TraitObligationStack<'o, 'tcx> {
write!(f, "TraitObligationStack({:?})", self.obligation)
}
}

pub enum ProjectionMatchesProjection {
Yes,
Ambiguous,
No,
}
1 change: 0 additions & 1 deletion src/test/ui/generic-associated-types/issue-74824.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ impl<T> 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(&copy) as *const _;
Expand Down
11 changes: 2 additions & 9 deletions src/test/ui/generic-associated-types/issue-74824.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,6 @@ help: consider restricting type parameter `T`
LL | type Copy<T: std::clone::Clone>: Copy = Box<T>;
| +++++++++++++++++++

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`.
35 changes: 35 additions & 0 deletions src/test/ui/generic-associated-types/issue-93874.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// check-pass

#![feature(generic_associated_types)]

pub trait Build {
type Output<O>;
fn build<O>(self, input: O) -> Self::Output<O>;
}

pub struct IdentityBuild;
impl Build for IdentityBuild {
type Output<O> = O;
fn build<O>(self, input: O) -> Self::Output<O> {
input
}
}

fn a() {
let _x: u8 = IdentityBuild.build(10);
}

fn b() {
let _x: Vec<u8> = IdentityBuild.build(Vec::new());
}

fn c() {
let mut f = IdentityBuild.build(|| ());
(f)();
}

pub fn main() {
a();
b();
c();
}

0 comments on commit 1e2f63d

Please sign in to comment.