about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJonas Schievink <jonasschievink@gmail.com>2021-01-24 22:10:01 +0100
committerGitHub <noreply@github.com>2021-01-24 22:10:01 +0100
commit70be327f2a1732e0e7bb464e067bcf8b8d0801dc (patch)
tree33befe417d225692f369e0e54a5c18955cc6eb70
parent504d6de52dfeab6086239180a92ff0bb5df2bb19 (diff)
parentf29b32983d1e885b0e141b6aac2cae281ff39a22 (diff)
downloadrust-70be327f2a1732e0e7bb464e067bcf8b8d0801dc.tar.gz
rust-70be327f2a1732e0e7bb464e067bcf8b8d0801dc.zip
Rollup merge of #81279 - bugadani:iter, r=davidtwco
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
-rw-r--r--compiler/rustc_typeck/src/check/check.rs242
1 files changed, 129 insertions, 113 deletions
diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs
index ab3c26fac83..47361092a5c 100644
--- a/compiler/rustc_typeck/src/check/check.rs
+++ b/compiler/rustc_typeck/src/check/check.rs
@@ -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 {
@@ -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.
@@ -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,
+                    );
                 }
             }
 
@@ -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)
@@ -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