Skip to content

Commit

Permalink
Rollup merge of #81279 - bugadani:iter, r=davidtwco
Browse files Browse the repository at this point in the history 
Small refactor in typeck

 - `check_impl_items_against_trait` only queries and walks through associated items once
 - extracted function that reports errors
 - don't check specialization validity when trait item does not match
 - small additional cleanups
  • Loading branch information
@jonas-schievink
jonas-schievink authored Jan 24, 2021
2 parents 504d6de + f29b329 commit 70be327
Showing 1 changed file with 129 additions and 113 deletions.
242 changes: 129 additions & 113 deletions compiler/rustc_typeck/src/check/check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,21 +846,13 @@ pub(super) fn check_specialization_validity<'tcx>(
Ok(ancestors) => ancestors,
Err(_) => return,
};
let mut ancestor_impls = ancestors
.skip(1)
.filter_map(|parent| {
if parent.is_from_trait() {
None
} else {
Some((parent, parent.item(tcx, trait_item.ident, kind, trait_def.def_id)))
}
})
.peekable();

if ancestor_impls.peek().is_none() {
// No parent, nothing to specialize.
return;
}
let mut ancestor_impls = ancestors.skip(1).filter_map(|parent| {
if parent.is_from_trait() {
None
} else {
Some((parent, parent.item(tcx, trait_item.ident, kind, trait_def.def_id)))
}
});

let opt_result = ancestor_impls.find_map(|(parent_impl, parent_item)| {
match parent_item {
Expand Down Expand Up @@ -902,8 +894,6 @@ pub(super) fn check_impl_items_against_trait<'tcx>(
impl_trait_ref: ty::TraitRef<'tcx>,
impl_item_refs: &[hir::ImplItemRef<'_>],
) {
let impl_span = tcx.sess.source_map().guess_head_span(full_impl_span);

// If the trait reference itself is erroneous (so the compilation is going
// to fail), skip checking the items here -- the `impl_item` table in `tcx`
// isn't populated for such impls.
Expand Down Expand Up @@ -931,111 +921,75 @@ pub(super) fn check_impl_items_against_trait<'tcx>(

// Locate trait definition and items
let trait_def = tcx.trait_def(impl_trait_ref.def_id);

let impl_items = || impl_item_refs.iter().map(|iiref| tcx.hir().impl_item(iiref.id));
let impl_items = impl_item_refs.iter().map(|iiref| tcx.hir().impl_item(iiref.id));
let associated_items = tcx.associated_items(impl_trait_ref.def_id);

// Check existing impl methods to see if they are both present in trait
// and compatible with trait signature
for impl_item in impl_items() {
let namespace = impl_item.kind.namespace();
for impl_item in impl_items {
let ty_impl_item = tcx.associated_item(tcx.hir().local_def_id(impl_item.hir_id));
let ty_trait_item = tcx
.associated_items(impl_trait_ref.def_id)
.find_by_name_and_namespace(tcx, ty_impl_item.ident, namespace, impl_trait_ref.def_id)
.or_else(|| {
// Not compatible, but needed for the error message
tcx.associated_items(impl_trait_ref.def_id)
.filter_by_name(tcx, ty_impl_item.ident, impl_trait_ref.def_id)
.next()
});

// Check that impl definition matches trait definition
if let Some(ty_trait_item) = ty_trait_item {

let mut items =
associated_items.filter_by_name(tcx, ty_impl_item.ident, impl_trait_ref.def_id);

let (compatible_kind, ty_trait_item) = if let Some(ty_trait_item) = items.next() {
let is_compatible = |ty: &&ty::AssocItem| match (ty.kind, &impl_item.kind) {
(ty::AssocKind::Const, hir::ImplItemKind::Const(..)) => true,
(ty::AssocKind::Fn, hir::ImplItemKind::Fn(..)) => true,
(ty::AssocKind::Type, hir::ImplItemKind::TyAlias(..)) => true,
_ => false,
};

// If we don't have a compatible item, we'll use the first one whose name matches
// to report an error.
let mut compatible_kind = is_compatible(&ty_trait_item);
let mut trait_item = ty_trait_item;

if !compatible_kind {
if let Some(ty_trait_item) = items.find(is_compatible) {
compatible_kind = true;
trait_item = ty_trait_item;
}
}

(compatible_kind, trait_item)
} else {
continue;
};

if compatible_kind {
match impl_item.kind {
hir::ImplItemKind::Const(..) => {
// Find associated const definition.
if ty_trait_item.kind == ty::AssocKind::Const {
compare_const_impl(
tcx,
&ty_impl_item,
impl_item.span,
&ty_trait_item,
impl_trait_ref,
);
} else {
let mut err = struct_span_err!(
tcx.sess,
impl_item.span,
E0323,
"item `{}` is an associated const, \
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");
// We can only get the spans from local trait definition
// Same for E0324 and E0325
if let Some(trait_span) = tcx.hir().span_if_local(ty_trait_item.def_id) {
err.span_label(trait_span, "item in trait");
}
err.emit()
}
compare_const_impl(
tcx,
&ty_impl_item,
impl_item.span,
&ty_trait_item,
impl_trait_ref,
);
}
hir::ImplItemKind::Fn(..) => {
let opt_trait_span = tcx.hir().span_if_local(ty_trait_item.def_id);
if ty_trait_item.kind == ty::AssocKind::Fn {
compare_impl_method(
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,
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) = opt_trait_span {
err.span_label(trait_span, "item in trait");
}
err.emit()
}
compare_impl_method(
tcx,
&ty_impl_item,
impl_item.span,
&ty_trait_item,
impl_trait_ref,
opt_trait_span,
);
}
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 {
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) = opt_trait_span {
err.span_label(trait_span, "item in trait");
}
err.emit()
}
compare_ty_impl(
tcx,
&ty_impl_item,
impl_item.span,
&ty_trait_item,
impl_trait_ref,
opt_trait_span,
);
}
}

Expand All @@ -1046,12 +1000,22 @@ pub(super) fn check_impl_items_against_trait<'tcx>(
impl_id.to_def_id(),
impl_item,
);
} else {
report_mismatch_error(
tcx,
ty_trait_item.def_id,
impl_trait_ref,
impl_item,
&ty_impl_item,
);
}
}

// Check for missing items from trait
let mut missing_items = Vec::new();
if let Ok(ancestors) = trait_def.ancestors(tcx, impl_id.to_def_id()) {
let impl_span = tcx.sess.source_map().guess_head_span(full_impl_span);

// Check for missing items from trait
let mut missing_items = Vec::new();
for trait_item in tcx.associated_items(impl_trait_ref.def_id).in_definition_order() {
let is_implemented = ancestors
.leaf_def(tcx, trait_item.ident, trait_item.kind)
Expand All @@ -1064,11 +1028,63 @@ pub(super) fn check_impl_items_against_trait<'tcx>(
}
}
}

if !missing_items.is_empty() {
missing_items_err(tcx, impl_span, &missing_items, full_impl_span);
}
}
}

#[inline(never)]
#[cold]
fn report_mismatch_error<'tcx>(
tcx: TyCtxt<'tcx>,
trait_item_def_id: DefId,
impl_trait_ref: ty::TraitRef<'tcx>,
impl_item: &hir::ImplItem<'_>,
ty_impl_item: &ty::AssocItem,
) {
let mut err = match impl_item.kind {
hir::ImplItemKind::Const(..) => {
// Find associated const definition.
struct_span_err!(
tcx.sess,
impl_item.span,
E0323,
"item `{}` is an associated const, which doesn't match its trait `{}`",
ty_impl_item.ident,
impl_trait_ref.print_only_trait_path()
)
}

hir::ImplItemKind::Fn(..) => {
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()
)
}

if !missing_items.is_empty() {
missing_items_err(tcx, impl_span, &missing_items, full_impl_span);
hir::ImplItemKind::TyAlias(_) => {
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(trait_item_def_id) {
err.span_label(trait_span, "item in trait");
}
err.emit();
}

/// Checks whether a type can be represented in memory. In particular, it
Expand Down

0 comments on commit 70be327

Please sign in to comment.