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.

Sign up for GitHub

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

Jump to bottom

Small refactor in typeck #81279

Merged
merged 7 commits into from
Jan 25, 2021
Merged

Small refactor in typeck #81279

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3e9f27d
Remove unnecessary closure
bugadani Jan 22, 2021
58a90de
No peeking
bugadani Jan 22, 2021
9988821
Move missing_item check inside condition
bugadani Jan 22, 2021
d63b278
Only scan through assoc items once in check_impl_items_against_trait
bugadani Jan 22, 2021
ee639de
Only guess span if absolutely necessary
bugadani Jan 22, 2021
aa4f583
Only query associated_items once
bugadani Jan 22, 2021
f29b329
Fix formatting
bugadani Jan 22, 2021
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
242 changes: 129 additions & 113 deletions compiler/rustc_typeck/src/check/check.rs
View file
Open in desktop
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