Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only mark projection as ambiguous if GAT substs are constrained #93892

Merged
merged 2 commits into from
Feb 19, 2022
Merged
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
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 @@ -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
}
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 @@ -1073,16 +1074,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 @@ -1180,7 +1171,7 @@ fn assemble_candidates_from_trait_def<'cx, 'tcx>(
ProjectionCandidate::TraitDef,
bounds.iter(),
true,
)
);
Copy link
Member

Choose a reason for hiding this comment

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

Is this diff new?

Copy link
Member Author

@compiler-errors compiler-errors Feb 15, 2022

Choose a reason for hiding this comment

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

function returns () and the rest have semicolons, but i'll double check

}

/// In the case of a trait object like
Expand Down Expand Up @@ -1245,28 +1236,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 @@ -1508,12 +1508,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(
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment is slightly outdated now that the return type has changed to an enum

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch.

&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 @@ -1535,7 +1541,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)
Expand All @@ -1545,7 +1552,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 @@ -2745,3 +2771,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();
}