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

Make GATs less ICE-prone. #67160

Merged
merged 8 commits into from
Dec 22, 2019
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: 2 additions & 0 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1912,6 +1912,7 @@ impl<'tcx> ObligationCause<'tcx> {
use crate::traits::ObligationCauseCode::*;
match self.code {
CompareImplMethodObligation { .. } => Error0308("method not compatible with trait"),
CompareImplTypeObligation { .. } => Error0308("type not compatible with trait"),
MatchExpressionArm(box MatchExpressionArmCause { source, .. }) =>
Error0308(match source {
hir::MatchSource::IfLetDesugar { .. } =>
Expand Down Expand Up @@ -1948,6 +1949,7 @@ impl<'tcx> ObligationCause<'tcx> {
use crate::traits::ObligationCauseCode::*;
match self.code {
CompareImplMethodObligation { .. } => "method type is compatible with trait",
CompareImplTypeObligation { .. } => "associated type is compatible with trait",
ExprAssignable => "expression is assignable",
MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => match source {
hir::MatchSource::IfLetDesugar { .. } => "`if let` arms have compatible types",
Expand Down
8 changes: 8 additions & 0 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
SelectionError::Unimplemented => {
if let ObligationCauseCode::CompareImplMethodObligation {
item_name, impl_item_def_id, trait_item_def_id,
} | ObligationCauseCode::CompareImplTypeObligation {
item_name, impl_item_def_id, trait_item_def_id,
} = obligation.cause.code {
self.report_extra_impl_obligation(
span,
Expand Down Expand Up @@ -2631,6 +2633,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
but not on the corresponding trait method",
predicate));
}
ObligationCauseCode::CompareImplTypeObligation { .. } => {
err.note(&format!(
"the requirement `{}` appears on the associated impl type\
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"the requirement `{}` appears on the associated impl type\
"the requirement `{}` appears on the associated impl type \

but not on the corresponding associated trait type",
predicate));
}
ObligationCauseCode::ReturnType |
ObligationCauseCode::ReturnValue(_) |
ObligationCauseCode::BlockTailExpression(_) => (),
Expand Down
7 changes: 7 additions & 0 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,13 @@ pub enum ObligationCauseCode<'tcx> {
trait_item_def_id: DefId,
},

/// Error derived when matching traits/impls; see ObligationCause for more details
CompareImplTypeObligation {
item_name: ast::Name,
impl_item_def_id: DefId,
trait_item_def_id: DefId,
},

/// Checking that this expression can be assigned where it needs to be
// FIXME(eddyb) #11161 is the original Expr required?
ExprAssignable,
Expand Down
26 changes: 21 additions & 5 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use crate::ty::subst::{Subst, InternalSubsts};
use crate::ty::{self, ToPredicate, ToPolyTraitRef, Ty, TyCtxt};
use crate::ty::fold::{TypeFoldable, TypeFolder};
use crate::util::common::FN_OUTPUT_NAME;
use syntax_pos::DUMMY_SP;

/// Depending on the stage of compilation, we want projection to be
/// more or less conservative.
Expand Down Expand Up @@ -1437,11 +1438,14 @@ fn confirm_impl_candidate<'cx, 'tcx>(
obligation: &ProjectionTyObligation<'tcx>,
impl_vtable: VtableImplData<'tcx, PredicateObligation<'tcx>>,
) -> Progress<'tcx> {
let tcx = selcx.tcx();

let VtableImplData { impl_def_id, substs, nested } = impl_vtable;
let assoc_item_id = obligation.predicate.item_def_id;
let trait_def_id = tcx.trait_id_of_impl(impl_def_id).unwrap();

let tcx = selcx.tcx();
let param_env = obligation.param_env;
let assoc_ty = assoc_ty_def(selcx, impl_def_id, obligation.predicate.item_def_id);
let assoc_ty = assoc_ty_def(selcx, impl_def_id, assoc_item_id);

if !assoc_ty.item.defaultness.has_value() {
// This means that the impl is missing a definition for the
Expand All @@ -1456,16 +1460,28 @@ fn confirm_impl_candidate<'cx, 'tcx>(
obligations: nested,
};
}
let substs = obligation.predicate.substs.rebase_onto(tcx, trait_def_id, substs);
let substs = translate_substs(selcx.infcx(), param_env, impl_def_id, substs, assoc_ty.node);
let ty = if let ty::AssocKind::OpaqueTy = assoc_ty.item.kind {
let item_substs = InternalSubsts::identity_for_item(tcx, assoc_ty.item.def_id);
tcx.mk_opaque(assoc_ty.item.def_id, item_substs)
} else {
tcx.type_of(assoc_ty.item.def_id)
};
Progress {
ty: ty.subst(tcx, substs),
obligations: nested,
if substs.len() != tcx.generics_of(assoc_ty.item.def_id).count() {
tcx.sess.delay_span_bug(
DUMMY_SP,
"impl item and trait item have different parameter counts",
);
Progress {
ty: tcx.types.err,
obligations: nested,
}
} else {
Progress {
ty: ty.subst(tcx, substs),
obligations: nested,
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,15 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
impl_item_def_id,
trait_item_def_id,
}),
super::CompareImplTypeObligation {
item_name,
impl_item_def_id,
trait_item_def_id,
} => Some(super::CompareImplTypeObligation {
item_name,
impl_item_def_id,
trait_item_def_id,
}),
super::ExprAssignable => Some(super::ExprAssignable),
super::MatchExpressionArm(box super::MatchExpressionArmCause {
arm_span,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1305,7 +1305,7 @@ impl<'tcx> PolyProjectionPredicate<'tcx> {
}

#[inline]
pub fn to_poly_trait_ref(&self, tcx: TyCtxt<'_>) -> PolyTraitRef<'tcx> {
pub fn to_poly_trait_ref(&self, tcx: TyCtxt<'tcx>) -> PolyTraitRef<'tcx> {
// Note: unlike with `TraitRef::to_poly_trait_ref()`,
// `self.0.trait_ref` is permitted to have escaping regions.
// This is because here `self` has a `Binder` and so does our
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1026,11 +1026,11 @@ impl<'tcx> ProjectionTy<'tcx> {
/// Extracts the underlying trait reference from this projection.
/// For example, if this is a projection of `<T as Iterator>::Item`,
/// then this function would return a `T: Iterator` trait reference.
pub fn trait_ref(&self, tcx: TyCtxt<'_>) -> ty::TraitRef<'tcx> {
pub fn trait_ref(&self, tcx: TyCtxt<'tcx>) -> ty::TraitRef<'tcx> {
let def_id = tcx.associated_item(self.item_def_id).container.id();
ty::TraitRef {
def_id,
substs: self.substs,
substs: self.substs.truncate_to(tcx, tcx.generics_of(def_id)),
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/librustc/ty/subst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl<'a, 'tcx> InternalSubsts<'tcx> {
ty::GenericParamDefKind::Const => {
tcx.mk_const(ty::Const {
val: ty::ConstKind::Bound(ty::INNERMOST, ty::BoundVar::from(param.index)),
ty: tcx.type_of(def_id),
ty: tcx.type_of(param.def_id),
}).into()
}
}
Expand Down Expand Up @@ -533,8 +533,7 @@ impl<'a, 'tcx> TypeFolder<'tcx> for SubstFolder<'a, 'tcx> {
data.name,
self.root_ty,
data.index);
self.tcx.sess.delay_span_bug(span, &msg);
r
span_bug!(span, "{}", msg);
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the need for this change? The delay_span_bug approach hides bugs elsewhere from us, but also safely lets the compiler continue for the users' sake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, it hides bugs.

}
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/librustc_resolve/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,17 +1119,16 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {

visit::walk_impl_item(this, impl_item);
}
AssocItemKind::TyAlias(_, Some(ref ty)) => {
AssocItemKind::TyAlias(_, _) => {
// If this is a trait impl, ensure the type
// exists in trait
this.check_trait_item(impl_item.ident,
TypeNS,
impl_item.span,
|n, s| TypeNotMemberOfTrait(n, s));

this.visit_ty(ty);
visit::walk_impl_item(this, impl_item);
matthewjasper marked this conversation as resolved.
Show resolved Hide resolved
}
AssocItemKind::TyAlias(_, None) => {}
AssocItemKind::Macro(_) =>
panic!("unexpanded macro in resolve!"),
}
Expand Down
86 changes: 70 additions & 16 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ pub trait AstConv<'tcx> {
fn projected_ty_from_poly_trait_ref(&self,
span: Span,
item_def_id: DefId,
item_segment: &hir::PathSegment,
poly_trait_ref: ty::PolyTraitRef<'tcx>)
-> Ty<'tcx>;

Expand Down Expand Up @@ -205,6 +206,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
let (substs, assoc_bindings, _) = self.create_substs_for_ast_path(
span,
def_id,
&[],
item_segment.generic_args(),
item_segment.infer_args,
None,
Expand Down Expand Up @@ -615,9 +617,21 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
/// `Output = u32` are returned in the `Vec<ConvertedBinding...>` result.
///
/// Note that the type listing given here is *exactly* what the user provided.
///
/// For (generic) associated types
///
/// ```
/// <Vec<u8> as Iterable<u8>>::Iter::<'a>
/// ```
///
/// We have the parent substs are the substs for the parent trait:
/// `[Vec<u8>, u8]` and `generic_args` are the arguments for the associated
/// type itself: `['a]`. The returned `SubstsRef` concatenates these two
/// lists: `[Vec<u8>, u8, 'a]`.
fn create_substs_for_ast_path<'a>(&self,
span: Span,
def_id: DefId,
parent_substs: &[subst::GenericArg<'tcx>],
matthewjasper marked this conversation as resolved.
Show resolved Hide resolved
generic_args: &'a hir::GenericArgs,
infer_args: bool,
self_ty: Option<Ty<'tcx>>)
Expand All @@ -633,17 +647,26 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
let tcx = self.tcx();
let generic_params = tcx.generics_of(def_id);

// If a self-type was declared, one should be provided.
assert_eq!(generic_params.has_self, self_ty.is_some());
if generic_params.has_self {
if generic_params.parent.is_some() {
// The parent is a trait so it should have at least one subst
// for the `Self` type.
assert!(!parent_substs.is_empty())
} else {
// This item (presumably a trait) needs a self-type.
assert!(self_ty.is_some());
}
} else {
assert!(self_ty.is_none() && parent_substs.is_empty());
}

let has_self = generic_params.has_self;
let (_, potential_assoc_types) = Self::check_generic_arg_count(
tcx,
span,
&generic_params,
&generic_args,
GenericArgPosition::Type,
has_self,
self_ty.is_some(),
infer_args,
);

Expand All @@ -652,7 +675,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
});
let default_needs_object_self = |param: &ty::GenericParamDef| {
if let GenericParamDefKind::Type { has_default, .. } = param.kind {
if is_object && has_default && has_self {
if is_object && has_default {
let self_param = tcx.types.self_param;
if tcx.at(span).type_of(param.def_id).walk().any(|ty| ty == self_param) {
// There is no suitable inference default for a type parameter
Expand All @@ -668,7 +691,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
let substs = Self::create_substs_for_generic_args(
tcx,
def_id,
&[][..],
parent_substs,
self_ty.is_some(),
self_ty,
// Provide the generic args, and whether types should be inferred.
Expand Down Expand Up @@ -780,6 +803,30 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
(substs, assoc_bindings, potential_assoc_types)
}

crate fn create_substs_for_associated_item(
&self,
tcx: TyCtxt<'tcx>,
span: Span,
item_def_id: DefId,
item_segment: &hir::PathSegment,
parent_substs: SubstsRef<'tcx>,
) -> SubstsRef<'tcx> {
if tcx.generics_of(item_def_id).params.is_empty() {
self.prohibit_generics(slice::from_ref(item_segment));

parent_substs
} else {
self.create_substs_for_ast_path(
span,
item_def_id,
parent_substs,
item_segment.generic_args(),
item_segment.infer_args,
None,
).0
}
}

/// Instantiates the path for the given trait reference, assuming that it's
/// bound to a valid trait type. Returns the `DefId` of the defining trait.
/// The type _cannot_ be a type other than a trait type.
Expand Down Expand Up @@ -919,6 +966,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {

self.create_substs_for_ast_path(span,
trait_def_id,
&[],
trait_segment.generic_args(),
trait_segment.infer_args,
Some(self_ty))
Expand Down Expand Up @@ -1665,8 +1713,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {

debug!("associated_path_to_ty: {:?}::{}", qself_ty, assoc_ident);

self.prohibit_generics(slice::from_ref(assoc_segment));

// Check if we have an enum variant.
let mut variant_resolution = None;
if let ty::Adt(adt_def, _) = qself_ty.kind {
Expand All @@ -1677,6 +1723,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
if let Some(variant_def) = variant_def {
if permit_variants {
tcx.check_stability(variant_def.def_id, Some(hir_ref_id), span);
self.prohibit_generics(slice::from_ref(assoc_segment));
return Ok((qself_ty, DefKind::Variant, variant_def.def_id));
} else {
variant_resolution = Some(variant_def.def_id);
Expand Down Expand Up @@ -1767,7 +1814,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
i.ident.modern() == assoc_ident
}).expect("missing associated type");

let ty = self.projected_ty_from_poly_trait_ref(span, item.def_id, bound);
let ty = self.projected_ty_from_poly_trait_ref(span, item.def_id, assoc_segment, bound);
let ty = self.normalize_ty(span, ty);

let kind = DefKind::AssocTy;
Expand Down Expand Up @@ -1818,8 +1865,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {

debug!("qpath_to_ty: trait_def_id={:?}", trait_def_id);

self.prohibit_generics(slice::from_ref(item_segment));

let self_ty = if let Some(ty) = opt_self_ty {
ty
} else {
Expand Down Expand Up @@ -1861,9 +1906,17 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
self_ty,
trait_segment);

let item_substs = self.create_substs_for_associated_item(
tcx,
span,
item_def_id,
item_segment,
trait_ref.substs,
);

debug!("qpath_to_ty: trait_ref={:?}", trait_ref);

self.normalize_ty(span, tcx.mk_projection(item_def_id, trait_ref.substs))
self.normalize_ty(span, tcx.mk_projection(item_def_id, item_substs))
}

pub fn prohibit_generics<'a, T: IntoIterator<Item = &'a hir::PathSegment>>(
Expand Down Expand Up @@ -2518,21 +2571,22 @@ impl<'tcx> Bounds<'tcx> {
// If it could be sized, and is, add the `Sized` predicate.
let sized_predicate = self.implicitly_sized.and_then(|span| {
tcx.lang_items().sized_trait().map(|sized| {
let trait_ref = ty::TraitRef {
let trait_ref = ty::Binder::bind(ty::TraitRef {
def_id: sized,
substs: tcx.mk_substs_trait(param_ty, &[])
};
});
(trait_ref.to_predicate(), span)
})
});

sized_predicate.into_iter().chain(
self.region_bounds.iter().map(|&(region_bound, span)| {
// Account for the binder being introduced below; no need to shift `param_ty`
// because, at present at least, it can only refer to early-bound regions.
// because, at present at least, it either only refers to early-bound regions,
// or it's a generic associated type that deliberately has escaping bound vars.
let region_bound = ty::fold::shift_region(tcx, region_bound, 1);
let outlives = ty::OutlivesPredicate(param_ty, region_bound);
(ty::Binder::dummy(outlives).to_predicate(), span)
(ty::Binder::bind(outlives).to_predicate(), span)
}).chain(
self.trait_bounds.iter().map(|&(bound_trait_ref, span)| {
(bound_trait_ref.to_predicate(), span)
Expand Down
Loading