Skip to content

Commit

Permalink
Check associated type implementations for generic mismatches
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewjasper committed Dec 20, 2019
1 parent 01a4650 commit db6d0b1
Show file tree
Hide file tree
Showing 6 changed files with 195 additions and 26 deletions.
2 changes: 2 additions & 0 deletions src/librustc/infer/error_reporting/mod.rs
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
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\
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
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
9 changes: 9 additions & 0 deletions src/librustc/traits/structural_impls.rs
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
176 changes: 155 additions & 21 deletions src/librustc_typeck/check/compare_method.rs
Expand Up @@ -5,7 +5,7 @@ use rustc::ty::{self, TyCtxt, GenericParamDefKind};
use rustc::ty::util::ExplicitSelf;
use rustc::traits::{self, ObligationCause, ObligationCauseCode, Reveal};
use rustc::ty::error::{ExpectedFound, TypeError};
use rustc::ty::subst::{Subst, InternalSubsts, SubstsRef};
use rustc::ty::subst::{Subst, InternalSubsts};
use rustc::util::common::ErrorReported;
use errors::{Applicability, DiagnosticId};

Expand All @@ -26,7 +26,7 @@ use rustc_error_codes::*;
/// - `trait_m`: the method in the trait
/// - `impl_trait_ref`: the TraitRef corresponding to the trait implementation

pub fn compare_impl_method<'tcx>(
crate fn compare_impl_method<'tcx>(
tcx: TyCtxt<'tcx>,
impl_m: &ty::AssocItem,
impl_m_span: Span,
Expand Down Expand Up @@ -181,13 +181,14 @@ fn compare_predicate_entailment<'tcx>(
let trait_m_predicates = tcx.predicates_of(trait_m.def_id);

// Check region bounds.
check_region_bounds_on_impl_method(tcx,
impl_m_span,
impl_m,
trait_m,
&trait_m_generics,
&impl_m_generics,
trait_to_skol_substs)?;
check_region_bounds_on_impl_item(
tcx,
impl_m_span,
impl_m,
trait_m,
&trait_m_generics,
&impl_m_generics,
)?;

// Create obligations for each predicate declared by the impl
// definition in the context of the trait's parameter
Expand Down Expand Up @@ -361,25 +362,22 @@ fn compare_predicate_entailment<'tcx>(
})
}

fn check_region_bounds_on_impl_method<'tcx>(
fn check_region_bounds_on_impl_item<'tcx>(
tcx: TyCtxt<'tcx>,
span: Span,
impl_m: &ty::AssocItem,
trait_m: &ty::AssocItem,
trait_generics: &ty::Generics,
impl_generics: &ty::Generics,
trait_to_skol_substs: SubstsRef<'tcx>,
) -> Result<(), ErrorReported> {
let trait_params = trait_generics.own_counts().lifetimes;
let impl_params = impl_generics.own_counts().lifetimes;

debug!("check_region_bounds_on_impl_method: \
debug!("check_region_bounds_on_impl_item: \
trait_generics={:?} \
impl_generics={:?} \
trait_to_skol_substs={:?}",
impl_generics={:?}",
trait_generics,
impl_generics,
trait_to_skol_substs);
impl_generics);

// Must have same number of early-bound lifetime parameters.
// Unfortunately, if the user screws up the bounds, then this
Expand All @@ -391,20 +389,25 @@ fn check_region_bounds_on_impl_method<'tcx>(
// are zero. Since I don't quite know how to phrase things at
// the moment, give a kind of vague error message.
if trait_params != impl_params {
let item_kind = assoc_item_kind_str(impl_m);
let def_span = tcx.sess.source_map().def_span(span);
let span = tcx.hir().get_generics(impl_m.def_id).map(|g| g.span).unwrap_or(def_span);
let mut err = struct_span_err!(
tcx.sess,
span,
E0195,
"lifetime parameters or bounds on method `{}` do not match the trait declaration",
"lifetime parameters or bounds on {} `{}` do not match the trait declaration",
item_kind,
impl_m.ident,
);
err.span_label(span, "lifetimes do not match method in trait");
err.span_label(span, &format!("lifetimes do not match {} in trait", item_kind));
if let Some(sp) = tcx.hir().span_if_local(trait_m.def_id) {
let def_sp = tcx.sess.source_map().def_span(sp);
let sp = tcx.hir().get_generics(trait_m.def_id).map(|g| g.span).unwrap_or(def_sp);
err.span_label(sp, "lifetimes in impl do not match this method in trait");
err.span_label(
sp,
&format!("lifetimes in impl do not match this {} in trait", item_kind),
);
}
err.emit();
return Err(ErrorReported);
Expand Down Expand Up @@ -603,6 +606,8 @@ fn compare_number_of_generics<'tcx>(
("const", trait_own_counts.consts, impl_own_counts.consts),
];

let item_kind = assoc_item_kind_str(impl_);

let mut err_occurred = false;
for &(kind, trait_count, impl_count) in &matchings {
if impl_count != trait_count {
Expand Down Expand Up @@ -647,8 +652,9 @@ fn compare_number_of_generics<'tcx>(
let mut err = tcx.sess.struct_span_err_with_code(
spans,
&format!(
"method `{}` has {} {kind} parameter{} but its trait \
"{} `{}` has {} {kind} parameter{} but its trait \
declaration has {} {kind} parameter{}",
item_kind,
trait_.ident,
impl_count,
pluralize!(impl_count),
Expand Down Expand Up @@ -961,7 +967,7 @@ fn compare_synthetic_generics<'tcx>(
}
}

pub fn compare_const_impl<'tcx>(
crate fn compare_const_impl<'tcx>(
tcx: TyCtxt<'tcx>,
impl_c: &ty::AssocItem,
impl_c_span: Span,
Expand Down Expand Up @@ -1059,3 +1065,131 @@ pub fn compare_const_impl<'tcx>(
fcx.regionck_item(impl_c_hir_id, impl_c_span, &[]);
});
}

crate fn compare_ty_impl<'tcx>(
tcx: TyCtxt<'tcx>,
impl_ty: &ty::AssocItem,
impl_ty_span: Span,
trait_ty: &ty::AssocItem,
impl_trait_ref: ty::TraitRef<'tcx>,
trait_item_span: Option<Span>,
) {
debug!("compare_impl_type(impl_trait_ref={:?})", impl_trait_ref);

let _: Result<(), ErrorReported> = (|| {
compare_number_of_generics(tcx, impl_ty, impl_ty_span, trait_ty, trait_item_span)?;

compare_type_predicate_entailment(tcx, impl_ty, impl_ty_span, trait_ty, impl_trait_ref)
})();
}

/// The equivalent of [compare_predicate_entailment], but for associated types
/// instead of associated functions.
fn compare_type_predicate_entailment(
tcx: TyCtxt<'tcx>,
impl_ty: &ty::AssocItem,
impl_ty_span: Span,
trait_ty: &ty::AssocItem,
impl_trait_ref: ty::TraitRef<'tcx>,
) -> Result<(), ErrorReported> {
let impl_substs = InternalSubsts::identity_for_item(tcx, impl_ty.def_id);
let trait_to_impl_substs = impl_substs.rebase_onto(tcx,
impl_ty.container.id(),
impl_trait_ref.substs);

let impl_ty_generics = tcx.generics_of(impl_ty.def_id);
let trait_ty_generics = tcx.generics_of(trait_ty.def_id);
let impl_ty_predicates = tcx.predicates_of(impl_ty.def_id);
let trait_ty_predicates = tcx.predicates_of(trait_ty.def_id);

check_region_bounds_on_impl_item(
tcx,
impl_ty_span,
impl_ty,
trait_ty,
&trait_ty_generics,
&impl_ty_generics,
)?;

let impl_ty_own_bounds = impl_ty_predicates.instantiate_own(tcx, impl_substs);

if impl_ty_own_bounds.is_empty() {
// Nothing to check.
return Ok(());
}

// This `HirId` should be used for the `body_id` field on each
// `ObligationCause` (and the `FnCtxt`). This is what
// `regionck_item` expects.
let impl_ty_hir_id = tcx.hir().as_local_hir_id(impl_ty.def_id).unwrap();
let cause = ObligationCause {
span: impl_ty_span,
body_id: impl_ty_hir_id,
code: ObligationCauseCode::CompareImplTypeObligation {
item_name: impl_ty.ident.name,
impl_item_def_id: impl_ty.def_id,
trait_item_def_id: trait_ty.def_id,
},
};

debug!("compare_type_predicate_entailment: trait_to_impl_substs={:?}", trait_to_impl_substs);

// The predicates declared by the impl definition, the trait and the
// associated type in the trait are assumed.
let impl_predicates = tcx.predicates_of(impl_ty_predicates.parent.unwrap());
let mut hybrid_preds = impl_predicates.instantiate_identity(tcx);
hybrid_preds.predicates.extend(
trait_ty_predicates.instantiate_own(tcx, trait_to_impl_substs).predicates);

debug!("compare_type_predicate_entailment: bounds={:?}", hybrid_preds);

let normalize_cause = traits::ObligationCause::misc(impl_ty_span, impl_ty_hir_id);
let param_env = ty::ParamEnv::new(
tcx.intern_predicates(&hybrid_preds.predicates),
Reveal::UserFacing,
None
);
let param_env = traits::normalize_param_env_or_error(tcx,
impl_ty.def_id,
param_env,
normalize_cause.clone());
tcx.infer_ctxt().enter(|infcx| {
let inh = Inherited::new(infcx, impl_ty.def_id);
let infcx = &inh.infcx;

debug!("compare_type_predicate_entailment: caller_bounds={:?}",
param_env.caller_bounds);

let mut selcx = traits::SelectionContext::new(&infcx);

for predicate in impl_ty_own_bounds.predicates {
let traits::Normalized { value: predicate, obligations } =
traits::normalize(&mut selcx, param_env, normalize_cause.clone(), &predicate);

inh.register_predicates(obligations);
inh.register_predicate(traits::Obligation::new(cause.clone(), param_env, predicate));
}

// Check that all obligations are satisfied by the implementation's
// version.
if let Err(ref errors) = inh.fulfillment_cx.borrow_mut().select_all_or_error(&infcx) {
infcx.report_fulfillment_errors(errors, None, false);
return Err(ErrorReported);
}

// Finally, resolve all regions. This catches wily misuses of
// lifetime parameters.
let fcx = FnCtxt::new(&inh, param_env, impl_ty_hir_id);
fcx.regionck_item(impl_ty_hir_id, impl_ty_span, &[]);

Ok(())
})
}

fn assoc_item_kind_str(impl_item: &ty::AssocItem) -> &'static str {
match impl_item.kind {
ty::AssocKind::Const => "const",
ty::AssocKind::Method => "method",
ty::AssocKind::Type | ty::AssocKind::OpaqueTy => "type",
}
}
19 changes: 14 additions & 5 deletions src/librustc_typeck/check/mod.rs
Expand Up @@ -154,7 +154,7 @@ pub use self::Expectation::*;
use self::autoderef::Autoderef;
use self::callee::DeferredCallResolution;
use self::coercion::{CoerceMany, DynamicCoerceMany};
pub use self::compare_method::{compare_impl_method, compare_const_impl};
use self::compare_method::{compare_impl_method, compare_const_impl, compare_ty_impl};
use self::method::{MethodCallee, SelfSource};
use self::TupleArgumentsFlag::*;

Expand Down Expand Up @@ -2014,41 +2014,50 @@ fn check_impl_items_against_trait<'tcx>(
}
}
hir::ImplItemKind::Method(..) => {
let trait_span = tcx.hir().span_if_local(ty_trait_item.def_id);
let opt_trait_span = tcx.hir().span_if_local(ty_trait_item.def_id);
if ty_trait_item.kind == ty::AssocKind::Method {
compare_impl_method(tcx,
&ty_impl_item,
impl_item.span,
&ty_trait_item,
impl_trait_ref,
trait_span);
opt_trait_span);
} else {
let mut err = struct_span_err!(tcx.sess, impl_item.span, E0324,
"item `{}` is an associated method, \
which doesn't match its trait `{}`",
ty_impl_item.ident,
impl_trait_ref.print_only_trait_path());
err.span_label(impl_item.span, "does not match trait");
if let Some(trait_span) = tcx.hir().span_if_local(ty_trait_item.def_id) {
if let Some(trait_span) = opt_trait_span {
err.span_label(trait_span, "item in trait");
}
err.emit()
}
}
hir::ImplItemKind::OpaqueTy(..) |
hir::ImplItemKind::TyAlias(_) => {
let opt_trait_span = tcx.hir().span_if_local(ty_trait_item.def_id);
if ty_trait_item.kind == ty::AssocKind::Type {
if ty_trait_item.defaultness.has_value() {
overridden_associated_type = Some(impl_item);
}
compare_ty_impl(
tcx,
&ty_impl_item,
impl_item.span,
&ty_trait_item,
impl_trait_ref,
opt_trait_span,
)
} else {
let mut err = struct_span_err!(tcx.sess, impl_item.span, E0325,
"item `{}` is an associated type, \
which doesn't match its trait `{}`",
ty_impl_item.ident,
impl_trait_ref.print_only_trait_path());
err.span_label(impl_item.span, "does not match trait");
if let Some(trait_span) = tcx.hir().span_if_local(ty_trait_item.def_id) {
if let Some(trait_span) = opt_trait_span {
err.span_label(trait_span, "item in trait");
}
err.emit()
Expand Down

0 comments on commit db6d0b1

Please sign in to comment.