Просмотр исходного кода

Diagnose impls that are fully overlapped by a final impl (#5417)

Such impls will never be used, so they should not exist. And test that a
final impl partially overlapping a non-final impl is accepted.

There is a question about a final impl partially overlapping a final
impl that is part of
https://github.com/carbon-language/carbon-lang/pull/5337
Dana Jansens 11 месяцев назад
Родитель
Сommit
b6a55c0818

+ 125 - 45
toolchain/check/check_unit.cpp

@@ -5,6 +5,7 @@
 #include "toolchain/check/check_unit.h"
 
 #include <string>
+#include <tuple>
 #include <utility>
 
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -570,46 +571,54 @@ auto CheckUnit::CheckPoisonedConcreteImplLookupQueries() -> void {
   context_.inst_block_stack().PopAndDiscard();
 }
 
-auto CheckUnit::CheckOverlappingImpls() -> void {
-  // Collect all of the impls sorted into contiguous segments by their
-  // interface. We only need to compare impls within each such segment.
-  llvm::SmallVector<SemIR::Impl> impls_by_interface(
-      context_.impls().array_ref());
-  llvm::stable_sort(
-      impls_by_interface, [](const SemIR::Impl& a, const SemIR::Impl& b) {
-        return a.interface.interface_id.index < b.interface.interface_id.index;
+// Check for invalid overlap between impls, given the set of all impls for a
+// single interface.
+static auto CheckOverlappingImplsForInterface(
+    Context& context,
+    llvm::ArrayRef<std::pair<SemIR::ImplId, SemIR::SpecificInterface>>
+        impls_and_interface) -> void {
+  // Range over `SemIR::ImplId` only. It'd be nice to make this the function
+  // parameter but we don't have a concept to express that outside the (banned)
+  // std::ranges.
+  auto impl_ids = llvm::map_range(
+      impls_and_interface,
+      [=](std::pair<SemIR::ImplId, SemIR::SpecificInterface> pair) {
+        auto [impl_id, _] = pair;
+        return impl_id;
       });
 
-  const auto* it = impls_by_interface.begin();
-  while (it != impls_by_interface.end()) {
-    const auto* segment_begin = it;
-    do {
-      ++it;
-    } while (it != impls_by_interface.end() &&
-             it->interface.interface_id ==
-                 segment_begin->interface.interface_id);
-    const auto* segment_end = it;
-
-    if (std::distance(segment_begin, segment_end) == 1) {
-      // Only 1 interface in the segment; nothing to overlap with.
-      continue;
-    }
-
-    CheckOverlappingImplsForInterface(
-        llvm::ArrayRef(segment_begin, segment_end));
-  }
-}
-
-auto CheckUnit::CheckOverlappingImplsForInterface(
-    llvm::ArrayRef<SemIR::Impl> impls) -> void {
-  for (auto [index_a, impl_a] : llvm::enumerate(impls)) {
+  // Avoid holding a reference into the ImplStore, as the diagnostic checks
+  // below can invalidate the reference. We copy out the part of the `Impl` we
+  // need.
+  struct ImplInfo {
+    bool is_final;
+    SemIR::InstId witness_id;
+    SemIR::TypeInstId self_id;
+    SemIR::InstId latest_decl_id;
+    SemIR::SpecificInterface interface;
+  };
+  auto get_impl_info = [&](SemIR::ImplId impl_id) -> ImplInfo {
+    const auto& impl = context.impls().Get(impl_id);
+    return {.is_final = impl.is_final,
+            .witness_id = impl.witness_id,
+            .self_id = impl.self_id,
+            .latest_decl_id = impl.latest_decl_id(),
+            .interface = impl.interface};
+  };
+
+  // TODO: We should revisit this and look for a way to do these checks in less
+  // than quadratic time. From @zygoloid: Possibly by converting the set of
+  // impls into a decision tree.
+  for (auto [index_a, impl_a_id] : llvm::enumerate(impl_ids)) {
+    auto impl_a = get_impl_info(impl_a_id);
     if (impl_a.witness_id == SemIR::ErrorInst::InstId) {
       continue;
     }
-    auto impl_a_type_structure =
-        BuildTypeStructure(context_, impl_a.self_id, impl_a.interface);
+    auto type_structure =
+        BuildTypeStructure(context, impl_a.self_id, impl_a.interface);
 
-    for (const auto& impl_b : impls.drop_front(index_a + 1)) {
+    for (auto impl_b_id : llvm::drop_begin(impl_ids, index_a + 1)) {
+      auto impl_b = get_impl_info(impl_b_id);
       if (impl_b.witness_id == SemIR::ErrorInst::InstId) {
         continue;
       }
@@ -617,20 +626,52 @@ auto CheckUnit::CheckOverlappingImplsForInterface(
       // The type structure each non-final `impl` must differ from all other
       // non-final `impl` for the same interface visible from the file.
       if (!impl_a.is_final && !impl_b.is_final) {
-        auto impl_b_type_structure =
-            BuildTypeStructure(context_, impl_b.self_id, impl_b.interface);
-        if (impl_a_type_structure == impl_b_type_structure) {
-          CARBON_DIAGNOSTIC(ImplFullyOverlapNonFinal, Error,
-                            "found non-final `impl` that fully overlaps "
-                            "previous non-final `impl`");
-          auto builder =
-              emitter_.Build(impl_b.latest_decl_id(), ImplFullyOverlapNonFinal);
-          CARBON_DIAGNOSTIC(ImplFullyOverlapNonFinalNote, Note,
-                            "fully overlaps `impl` here");
-          builder.Note(impl_a.latest_decl_id(), ImplFullyOverlapNonFinalNote);
+        auto type_structure2 =
+            BuildTypeStructure(context, impl_b.self_id, impl_b.interface);
+        if (type_structure == type_structure2) {
+          CARBON_DIAGNOSTIC(ImplNonFinalSameTypeStructure, Error,
+                            "found non-final `impl` with the same type "
+                            "structure as another non-final `impl`");
+          auto builder = context.emitter().Build(impl_b.latest_decl_id,
+                                                 ImplNonFinalSameTypeStructure);
+          CARBON_DIAGNOSTIC(ImplNonFinalSameTypeStructureNote, Note,
+                            "other `impl` here");
+          builder.Note(impl_a.latest_decl_id,
+                       ImplNonFinalSameTypeStructureNote);
           builder.Emit();
           break;
         }
+      } else {
+        CARBON_CHECK(impl_a.is_final || impl_b.is_final);
+
+        auto diagnose = [&](ImplInfo& query_impl, SemIR::ImplId final_impl_id,
+                            const ImplInfo& final_impl) -> bool {
+          if (LookupMatchesImpl(
+                  context, SemIR::LocId(query_impl.latest_decl_id),
+                  context.constant_values().Get(query_impl.self_id),
+                  query_impl.interface, final_impl_id)) {
+            CARBON_DIAGNOSTIC(ImplFinalOverlapsNonFinal, Error,
+                              "`impl` will never be used");
+            auto builder = context.emitter().Build(query_impl.latest_decl_id,
+                                                   ImplFinalOverlapsNonFinal);
+            CARBON_DIAGNOSTIC(
+                ImplFinalOverlapsNonFinalNote, Note,
+                "`final impl` declared here would always be used instead");
+            builder.Note(final_impl.latest_decl_id,
+                         ImplFinalOverlapsNonFinalNote);
+            builder.Emit();
+            return true;
+          }
+          return false;
+        };
+
+        bool did_diagnose = false;
+        if (impl_a.is_final) {
+          did_diagnose = diagnose(impl_b, impl_a_id, impl_a);
+        }
+        if (impl_b.is_final && !did_diagnose) {
+          diagnose(impl_a, impl_b_id, impl_b);
+        }
       }
     }
 
@@ -640,6 +681,45 @@ auto CheckUnit::CheckOverlappingImplsForInterface(
   }
 }
 
+auto CheckUnit::CheckOverlappingImpls() -> void {
+  // Collect all of the impls sorted into contiguous segments by their
+  // interface. We only need to compare impls within each such segment.
+  //
+  // Don't hold Impl pointers here because the process of looking for
+  // diagnostics may cause imports and may invalidate pointers into the
+  // ImplStore.
+  llvm::SmallVector<std::pair<SemIR::ImplId, SemIR::SpecificInterface>>
+      impl_ids_by_interface(llvm::map_range(
+          context_.impls().enumerate(),
+          [](std::pair<SemIR::ImplId, const SemIR::Impl&> pair) {
+            return std::make_pair(pair.first, pair.second.interface);
+          }));
+  llvm::stable_sort(
+      impl_ids_by_interface,
+      [](std::pair<SemIR::ImplId, SemIR::SpecificInterface> a,
+         std::pair<SemIR::ImplId, const SemIR::SpecificInterface> b) {
+        return a.second.interface_id.index < b.second.interface_id.index;
+      });
+
+  const auto* it = impl_ids_by_interface.begin();
+  while (it != impl_ids_by_interface.end()) {
+    const auto* segment_begin = it;
+    do {
+      ++it;
+    } while (it != impl_ids_by_interface.end() &&
+             it->second.interface_id == segment_begin->second.interface_id);
+    const auto* segment_end = it;
+
+    if (std::distance(segment_begin, segment_end) == 1) {
+      // Only 1 interface in the segment; nothing to overlap with.
+      continue;
+    }
+
+    CheckOverlappingImplsForInterface(
+        context_, llvm::ArrayRef(segment_begin, segment_end));
+  }
+}
+
 auto CheckUnit::FinishRun() -> void {
   CheckRequiredDeclarations();
   CheckRequiredDefinitions();

+ 0 - 4
toolchain/check/check_unit.h

@@ -176,10 +176,6 @@ class CheckUnit {
   // - The type structure each non-final `impl` must differ from every other
   //   non-final `impl` for the same interface visible from the file.
   auto CheckOverlappingImpls() -> void;
-  // Check for invalid overlap between impls, given the set of all impls for a
-  // single interface.
-  auto CheckOverlappingImplsForInterface(llvm::ArrayRef<SemIR::Impl> impls)
-      -> void;
 
   // Does work after processing the parse tree, such as finishing the IR and
   // checking for missing contents.

+ 13 - 0
toolchain/check/impl_lookup.cpp

@@ -690,4 +690,17 @@ auto EvalLookupSingleImplWitness(Context& context, SemIR::LocId loc_id,
   return EvalImplLookupResult::MakeNone();
 }
 
+auto LookupMatchesImpl(Context& context, SemIR::LocId loc_id,
+                       SemIR::ConstantId query_self_const_id,
+                       SemIR::SpecificInterface query_specific_interface,
+                       SemIR::ImplId target_impl) -> bool {
+  if (query_self_const_id == SemIR::ErrorInst::ConstantId) {
+    return false;
+  }
+  auto result = GetWitnessIdForImpl(
+      context, loc_id, /*query_is_concrete=*/false, query_self_const_id,
+      query_specific_interface, target_impl);
+  return result.has_value();
+}
+
 }  // namespace Carbon::Check

+ 8 - 0
toolchain/check/impl_lookup.h

@@ -39,6 +39,14 @@ auto LookupImplWitness(Context& context, SemIR::LocId loc_id,
                        SemIR::ConstantId query_facet_type_const_id)
     -> SemIR::InstBlockIdOrError;
 
+// Returns whether the query matches against the given impl. This is like a
+// `LookupImplWitness` operation but for a single interface, and against only
+// the single impl.
+auto LookupMatchesImpl(Context& context, SemIR::LocId loc_id,
+                       SemIR::ConstantId query_self_const_id,
+                       SemIR::SpecificInterface query_specific_interface,
+                       SemIR::ImplId target_impl) -> bool;
+
 // The result of EvalLookupSingleImplWitness(). It can be one of:
 // - No value. Lookup failed to find an impl declaration.
 // - A concrete value. Lookup found a concrete impl declaration that can be

+ 4 - 4
toolchain/check/testdata/impl/lookup/min_prelude/impl_cycle.carbon

@@ -269,19 +269,19 @@ impl C as ComparableTo(D) {}
 // T is always ComparableWith(T).
 impl forall [T:! type] T as ComparableWith(T) {}
 // If U is ComparableTo(T), U is ComparableWith(T).
-// CHECK:STDERR: fail_todo_impl_recurse_with_simpler_type_in_generic_param_bidirectional_no_cycle.carbon:[[@LINE+7]]:1: error: found non-final `impl` that fully overlaps previous non-final `impl` [ImplFullyOverlapNonFinal]
+// CHECK:STDERR: fail_todo_impl_recurse_with_simpler_type_in_generic_param_bidirectional_no_cycle.carbon:[[@LINE+7]]:1: error: found non-final `impl` with the same type structure as another non-final `impl` [ImplNonFinalSameTypeStructure]
 // CHECK:STDERR: impl forall [T:! type, U:! ComparableTo(T)] U as ComparableWith(T) {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_impl_recurse_with_simpler_type_in_generic_param_bidirectional_no_cycle.carbon:[[@LINE-5]]:1: note: fully overlaps `impl` here [ImplFullyOverlapNonFinalNote]
+// CHECK:STDERR: fail_todo_impl_recurse_with_simpler_type_in_generic_param_bidirectional_no_cycle.carbon:[[@LINE-5]]:1: note: other `impl` here [ImplNonFinalSameTypeStructureNote]
 // CHECK:STDERR: impl forall [T:! type] T as ComparableWith(T) {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:
 impl forall [T:! type, U:! ComparableTo(T)] U as ComparableWith(T) {}
 // If U is ComparableTo(T), T is ComparableWith(U).
-// CHECK:STDERR: fail_todo_impl_recurse_with_simpler_type_in_generic_param_bidirectional_no_cycle.carbon:[[@LINE+7]]:1: error: found non-final `impl` that fully overlaps previous non-final `impl` [ImplFullyOverlapNonFinal]
+// CHECK:STDERR: fail_todo_impl_recurse_with_simpler_type_in_generic_param_bidirectional_no_cycle.carbon:[[@LINE+7]]:1: error: found non-final `impl` with the same type structure as another non-final `impl` [ImplNonFinalSameTypeStructure]
 // CHECK:STDERR: impl forall [T:! type, U:! ComparableTo(T)] T as ComparableWith(U) {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_impl_recurse_with_simpler_type_in_generic_param_bidirectional_no_cycle.carbon:[[@LINE-5]]:1: note: fully overlaps `impl` here [ImplFullyOverlapNonFinalNote]
+// CHECK:STDERR: fail_todo_impl_recurse_with_simpler_type_in_generic_param_bidirectional_no_cycle.carbon:[[@LINE-5]]:1: note: other `impl` here [ImplNonFinalSameTypeStructureNote]
 // CHECK:STDERR: impl forall [T:! type, U:! ComparableTo(T)] U as ComparableWith(T) {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:

+ 87 - 15
toolchain/check/testdata/impl/lookup/min_prelude/specialization_poison.carbon

@@ -170,49 +170,77 @@ final impl forall [V:! type] V* as I where .T = V {
   fn F[self: Self]() -> V { return *self; }
 }
 
-// --- todo_final_overlaps_earlier_non_final_impl.carbon
+// --- fail_final_overlaps_earlier_non_final_impl.carbon
 library "[[@TEST_NAME]]";
 
 interface Z(T:! type) {}
 interface Y {}
 
-impl forall [T:! Y] T as Z(T) {}
+// CHECK:STDERR: fail_final_overlaps_earlier_non_final_impl.carbon:[[@LINE+3]]:1: error: `impl` will never be used [ImplFinalOverlapsNonFinal]
+// CHECK:STDERR: impl () as Z(()) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
+impl () as Z(()) {}
 
-// TODO: Diagnose overlap with previous impl (query of this matches that).
+// CHECK:STDERR: fail_final_overlaps_earlier_non_final_impl.carbon:[[@LINE+4]]:1: note: `final impl` declared here would always be used instead [ImplFinalOverlapsNonFinalNote]
+// CHECK:STDERR: final impl forall [T:! type] T as Z(T) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
 final impl forall [T:! type] T as Z(T) {}
 
-// --- todo_final_overlaps_later_non_final_impl.carbon
+// --- fail_final_overlaps_later_non_final_impl.carbon
 library "[[@TEST_NAME]]";
 
 interface Z(T:! type) {}
 interface Y {}
 
-// TODO: Diagnose overlap with next impl (query of this matches that).
 final impl forall [T:! type] T as Z(T) {}
 
-impl forall [T:! Y] T as Z(T) {}
+// CHECK:STDERR: fail_final_overlaps_later_non_final_impl.carbon:[[@LINE+7]]:1: error: `impl` will never be used [ImplFinalOverlapsNonFinal]
+// CHECK:STDERR: impl () as Z(()) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_final_overlaps_later_non_final_impl.carbon:[[@LINE-5]]:1: note: `final impl` declared here would always be used instead [ImplFinalOverlapsNonFinalNote]
+// CHECK:STDERR: final impl forall [T:! type] T as Z(T) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+impl () as Z(()) {}
 
-// --- todo_final_overlaps_earlier_final_impl.carbon
+// --- fail_final_overlaps_earlier_final_impl.carbon
 library "[[@TEST_NAME]]";
 
 interface Z(T:! type) {}
 interface Y {}
 
-final impl forall [T:! Y] T as Z(T) {}
+class C;
+
+// CHECK:STDERR: fail_final_overlaps_earlier_final_impl.carbon:[[@LINE+3]]:1: error: `impl` will never be used [ImplFinalOverlapsNonFinal]
+// CHECK:STDERR: final impl C as Z(C) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~
+final impl C as Z(C) {}
 
-// TODO: Diagnose overlap with previous impl (query of this matches that).
+// CHECK:STDERR: fail_final_overlaps_earlier_final_impl.carbon:[[@LINE+4]]:1: note: `final impl` declared here would always be used instead [ImplFinalOverlapsNonFinalNote]
+// CHECK:STDERR: final impl forall [T:! type] T as Z(T) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
 final impl forall [T:! type] T as Z(T) {}
 
-// --- todo_final_overlaps_later_final_impl.carbon
+// --- fail_final_overlaps_later_final_impl.carbon
 library "[[@TEST_NAME]]";
 
 interface Z(T:! type) {}
 interface Y {}
 
-// TODO: Diagnose overlap with next impl (query of this matches that)
+class C;
+
 final impl forall [T:! type] T as Z(T) {}
 
-final impl forall [T:! Y] T as Z(T) {}
+// CHECK:STDERR: fail_final_overlaps_later_final_impl.carbon:[[@LINE+7]]:1: error: `impl` will never be used [ImplFinalOverlapsNonFinal]
+// CHECK:STDERR: final impl C as Z(C) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_final_overlaps_later_final_impl.carbon:[[@LINE-5]]:1: note: `final impl` declared here would always be used instead [ImplFinalOverlapsNonFinalNote]
+// CHECK:STDERR: final impl forall [T:! type] T as Z(T) {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+final impl C as Z(C) {}
 
 // --- fail_non_final_overlaps_non_final_impl.carbon
 library "[[@TEST_NAME]]";
@@ -222,14 +250,15 @@ interface Y {}
 
 impl forall [T:! Y] T as Z(T) {}
 
-class C(T:! type) {}
+class C(T:! type);
+
 // No diagnosis here as the type structure is unique.
 impl forall [T:! type] T as Z(C(T)) {}
 
-// CHECK:STDERR: fail_non_final_overlaps_non_final_impl.carbon:[[@LINE+7]]:1: error: found non-final `impl` that fully overlaps previous non-final `impl` [ImplFullyOverlapNonFinal]
+// CHECK:STDERR: fail_non_final_overlaps_non_final_impl.carbon:[[@LINE+7]]:1: error: found non-final `impl` with the same type structure as another non-final `impl` [ImplNonFinalSameTypeStructure]
 // CHECK:STDERR: impl forall [T:! type] T as Z(T) {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_non_final_overlaps_non_final_impl.carbon:[[@LINE-9]]:1: note: fully overlaps `impl` here [ImplFullyOverlapNonFinalNote]
+// CHECK:STDERR: fail_non_final_overlaps_non_final_impl.carbon:[[@LINE-10]]:1: note: other `impl` here [ImplNonFinalSameTypeStructureNote]
 // CHECK:STDERR: impl forall [T:! Y] T as Z(T) {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:
@@ -245,3 +274,46 @@ class D;
 
 impl C as Z {}
 impl D as Z {}
+
+// --- final_partial_overlaps_non_final_impl.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {}
+interface Y {}
+
+class C;
+
+impl forall [T:! type] T as Z(T) {}
+
+// Partially overlaps the previous impl, which is fine.
+final impl C as Z(C) {}
+
+// --- todo_final_partial_overlaps_final_impl_in_self_type.carbon
+library "[[@TEST_NAME]]";
+
+interface Z {}
+interface Y {}
+
+class C(T:! type, U:! type);
+
+final impl forall [T:! type] C(T, ()) as Z {}
+
+// TODO: Partially overlaps the previous impl, which may or may not be fine outside of
+// a `match_first`, depending how
+// https://github.com/carbon-language/carbon-lang/pull/5337 turns out.
+final impl forall [U:! type] C((), U) as Z {}
+
+// --- todo_final_partial_overlaps_final_impl_in_interface.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type, U:! type) {}
+interface Y {}
+
+class C;
+
+final impl forall [T:! type] C as Z(T, ()) {}
+
+// TODO: Partially overlaps the previous impl, which may or may not be fine outside of
+// a `match_first`, depending how
+// https://github.com/carbon-language/carbon-lang/pull/5337 turns out.
+final impl forall [U:! type] C as Z((), U) {}

+ 105 - 104
toolchain/check/testdata/impl/min_prelude/generic_redeclaration.carbon

@@ -11,7 +11,7 @@
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/min_prelude/generic_redeclaration.carbon
 
-// --- fail_same_self_and_interface.carbon
+// --- fail_todo_same_self_and_interface.carbon
 
 library "[[@TEST_NAME]]";
 
@@ -22,6 +22,7 @@ interface J {}
 interface K {}
 interface L {}
 
+// TODO: Put these in a match_first so the test will pass again.
 impl forall [T:! I] T as Interface;
 impl forall [T:! J] T as Interface;
 impl forall [T:! K] T as Interface;
@@ -30,26 +31,26 @@ impl forall [T:! L] T as Interface;
 // These are different impls, so they are not redefinitions, even though the
 // self type and constraint type are the same.
 impl forall [T:! I] T as Interface {}
-// CHECK:STDERR: fail_same_self_and_interface.carbon:[[@LINE+7]]:1: error: found non-final `impl` that fully overlaps previous non-final `impl` [ImplFullyOverlapNonFinal]
+// CHECK:STDERR: fail_todo_same_self_and_interface.carbon:[[@LINE+7]]:1: error: found non-final `impl` with the same type structure as another non-final `impl` [ImplNonFinalSameTypeStructure]
 // CHECK:STDERR: impl forall [T:! J] T as Interface {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_same_self_and_interface.carbon:[[@LINE-4]]:1: note: fully overlaps `impl` here [ImplFullyOverlapNonFinalNote]
+// CHECK:STDERR: fail_todo_same_self_and_interface.carbon:[[@LINE-4]]:1: note: other `impl` here [ImplNonFinalSameTypeStructureNote]
 // CHECK:STDERR: impl forall [T:! I] T as Interface {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:
 impl forall [T:! J] T as Interface {}
-// CHECK:STDERR: fail_same_self_and_interface.carbon:[[@LINE+7]]:1: error: found non-final `impl` that fully overlaps previous non-final `impl` [ImplFullyOverlapNonFinal]
+// CHECK:STDERR: fail_todo_same_self_and_interface.carbon:[[@LINE+7]]:1: error: found non-final `impl` with the same type structure as another non-final `impl` [ImplNonFinalSameTypeStructure]
 // CHECK:STDERR: impl forall [T:! K] T as Interface {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_same_self_and_interface.carbon:[[@LINE-4]]:1: note: fully overlaps `impl` here [ImplFullyOverlapNonFinalNote]
+// CHECK:STDERR: fail_todo_same_self_and_interface.carbon:[[@LINE-4]]:1: note: other `impl` here [ImplNonFinalSameTypeStructureNote]
 // CHECK:STDERR: impl forall [T:! J] T as Interface {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:
 impl forall [T:! K] T as Interface {}
-// CHECK:STDERR: fail_same_self_and_interface.carbon:[[@LINE+7]]:1: error: found non-final `impl` that fully overlaps previous non-final `impl` [ImplFullyOverlapNonFinal]
+// CHECK:STDERR: fail_todo_same_self_and_interface.carbon:[[@LINE+7]]:1: error: found non-final `impl` with the same type structure as another non-final `impl` [ImplNonFinalSameTypeStructure]
 // CHECK:STDERR: impl forall [T:! L] T as Interface {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_same_self_and_interface.carbon:[[@LINE-4]]:1: note: fully overlaps `impl` here [ImplFullyOverlapNonFinalNote]
+// CHECK:STDERR: fail_todo_same_self_and_interface.carbon:[[@LINE-4]]:1: note: other `impl` here [ImplNonFinalSameTypeStructureNote]
 // CHECK:STDERR: impl forall [T:! K] T as Interface {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:
@@ -82,10 +83,10 @@ interface I {}
 // Since the spelling is different, it's not caught as a redefinition, but it
 // is diagnosed as an overlapping impl without using a `match_first` block.
 impl C as I {}
-// CHECK:STDERR: fail_same_type_different_spelling.carbon:[[@LINE+7]]:1: error: found non-final `impl` that fully overlaps previous non-final `impl` [ImplFullyOverlapNonFinal]
+// CHECK:STDERR: fail_same_type_different_spelling.carbon:[[@LINE+7]]:1: error: found non-final `impl` with the same type structure as another non-final `impl` [ImplNonFinalSameTypeStructure]
 // CHECK:STDERR: impl (C, C).0 as I {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_same_type_different_spelling.carbon:[[@LINE-4]]:1: note: fully overlaps `impl` here [ImplFullyOverlapNonFinalNote]
+// CHECK:STDERR: fail_same_type_different_spelling.carbon:[[@LINE-4]]:1: note: other `impl` here [ImplNonFinalSameTypeStructureNote]
 // CHECK:STDERR: impl C as I {}
 // CHECK:STDERR: ^~~~~~~~~~~~~
 // CHECK:STDERR:
@@ -116,7 +117,7 @@ impl forall [T:! type] T as I {
   }
 }
 
-// CHECK:STDOUT: --- fail_same_self_and_interface.carbon
+// CHECK:STDOUT: --- fail_todo_same_self_and_interface.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Interface.type: type = facet_type <@Interface> [concrete]
@@ -132,19 +133,19 @@ impl forall [T:! type] T as I {
 // CHECK:STDOUT:   %T.826: %I.type = bind_symbolic_name T, 0 [symbolic]
 // CHECK:STDOUT:   %pattern_type.2b5: type = pattern_type %I.type [concrete]
 // CHECK:STDOUT:   %T.as_type.b70: type = facet_access_type %T.826 [symbolic]
-// CHECK:STDOUT:   %Interface.impl_witness.a14: <witness> = impl_witness file.%Interface.impl_witness_table.loc11, @impl.c3d(%T.826) [symbolic]
+// CHECK:STDOUT:   %Interface.impl_witness.a14: <witness> = impl_witness file.%Interface.impl_witness_table.loc12, @impl.c3d(%T.826) [symbolic]
 // CHECK:STDOUT:   %T.ccd: %J.type = bind_symbolic_name T, 0 [symbolic]
 // CHECK:STDOUT:   %pattern_type.28e: type = pattern_type %J.type [concrete]
 // CHECK:STDOUT:   %T.as_type.3df: type = facet_access_type %T.ccd [symbolic]
-// CHECK:STDOUT:   %Interface.impl_witness.608: <witness> = impl_witness file.%Interface.impl_witness_table.loc12, @impl.793(%T.ccd) [symbolic]
+// CHECK:STDOUT:   %Interface.impl_witness.608: <witness> = impl_witness file.%Interface.impl_witness_table.loc13, @impl.793(%T.ccd) [symbolic]
 // CHECK:STDOUT:   %T.09f: %K.type = bind_symbolic_name T, 0 [symbolic]
 // CHECK:STDOUT:   %pattern_type.4a6: type = pattern_type %K.type [concrete]
 // CHECK:STDOUT:   %T.as_type.037: type = facet_access_type %T.09f [symbolic]
-// CHECK:STDOUT:   %Interface.impl_witness.b9e: <witness> = impl_witness file.%Interface.impl_witness_table.loc13, @impl.c93(%T.09f) [symbolic]
+// CHECK:STDOUT:   %Interface.impl_witness.b9e: <witness> = impl_witness file.%Interface.impl_witness_table.loc14, @impl.c93(%T.09f) [symbolic]
 // CHECK:STDOUT:   %T.1d2: %L.type = bind_symbolic_name T, 0 [symbolic]
 // CHECK:STDOUT:   %pattern_type.86b: type = pattern_type %L.type [concrete]
 // CHECK:STDOUT:   %T.as_type.0ed: type = facet_access_type %T.1d2 [symbolic]
-// CHECK:STDOUT:   %Interface.impl_witness.e9d: <witness> = impl_witness file.%Interface.impl_witness_table.loc14, @impl.9e6(%T.1d2) [symbolic]
+// CHECK:STDOUT:   %Interface.impl_witness.e9d: <witness> = impl_witness file.%Interface.impl_witness_table.loc15, @impl.9e6(%T.1d2) [symbolic]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -171,90 +172,90 @@ impl forall [T:! type] T as I {
 // CHECK:STDOUT:   impl_decl @impl.c3d [concrete] {
 // CHECK:STDOUT:     %T.patt: %pattern_type.2b5 = symbolic_binding_pattern T, 0 [concrete]
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %T.ref.loc11: %I.type = name_ref T, %T.loc11_14.1 [symbolic = %T.loc11_14.2 (constants.%T.826)]
-// CHECK:STDOUT:     %T.as_type.loc11_21.1: type = facet_access_type %T.ref.loc11 [symbolic = %T.as_type.loc11_21.2 (constants.%T.as_type.b70)]
-// CHECK:STDOUT:     %.loc11: type = converted %T.ref.loc11, %T.as_type.loc11_21.1 [symbolic = %T.as_type.loc11_21.2 (constants.%T.as_type.b70)]
-// CHECK:STDOUT:     %Interface.ref.loc11: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
-// CHECK:STDOUT:     %I.ref.loc11: type = name_ref I, file.%I.decl [concrete = constants.%I.type]
-// CHECK:STDOUT:     %T.loc11_14.1: %I.type = bind_symbolic_name T, 0 [symbolic = %T.loc11_14.2 (constants.%T.826)]
-// CHECK:STDOUT:   }
-// CHECK:STDOUT:   %Interface.impl_witness_table.loc11 = impl_witness_table (), @impl.c3d [concrete]
-// CHECK:STDOUT:   %Interface.impl_witness.loc11: <witness> = impl_witness %Interface.impl_witness_table.loc11, @impl.c3d(constants.%T.826) [symbolic = @impl.c3d.%Interface.impl_witness (constants.%Interface.impl_witness.a14)]
+// CHECK:STDOUT:     %T.ref.loc12: %I.type = name_ref T, %T.loc12_14.1 [symbolic = %T.loc12_14.2 (constants.%T.826)]
+// CHECK:STDOUT:     %T.as_type.loc12_21.1: type = facet_access_type %T.ref.loc12 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.b70)]
+// CHECK:STDOUT:     %.loc12: type = converted %T.ref.loc12, %T.as_type.loc12_21.1 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.b70)]
+// CHECK:STDOUT:     %Interface.ref.loc12: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
+// CHECK:STDOUT:     %I.ref.loc12: type = name_ref I, file.%I.decl [concrete = constants.%I.type]
+// CHECK:STDOUT:     %T.loc12_14.1: %I.type = bind_symbolic_name T, 0 [symbolic = %T.loc12_14.2 (constants.%T.826)]
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Interface.impl_witness_table.loc12 = impl_witness_table (), @impl.c3d [concrete]
+// CHECK:STDOUT:   %Interface.impl_witness.loc12: <witness> = impl_witness %Interface.impl_witness_table.loc12, @impl.c3d(constants.%T.826) [symbolic = @impl.c3d.%Interface.impl_witness (constants.%Interface.impl_witness.a14)]
 // CHECK:STDOUT:   impl_decl @impl.793 [concrete] {
 // CHECK:STDOUT:     %T.patt: %pattern_type.28e = symbolic_binding_pattern T, 0 [concrete]
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %T.ref.loc12: %J.type = name_ref T, %T.loc12_14.1 [symbolic = %T.loc12_14.2 (constants.%T.ccd)]
-// CHECK:STDOUT:     %T.as_type.loc12_21.1: type = facet_access_type %T.ref.loc12 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.3df)]
-// CHECK:STDOUT:     %.loc12: type = converted %T.ref.loc12, %T.as_type.loc12_21.1 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.3df)]
-// CHECK:STDOUT:     %Interface.ref.loc12: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
-// CHECK:STDOUT:     %J.ref.loc12: type = name_ref J, file.%J.decl [concrete = constants.%J.type]
-// CHECK:STDOUT:     %T.loc12_14.1: %J.type = bind_symbolic_name T, 0 [symbolic = %T.loc12_14.2 (constants.%T.ccd)]
+// CHECK:STDOUT:     %T.ref.loc13: %J.type = name_ref T, %T.loc13_14.1 [symbolic = %T.loc13_14.2 (constants.%T.ccd)]
+// CHECK:STDOUT:     %T.as_type.loc13_21.1: type = facet_access_type %T.ref.loc13 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.3df)]
+// CHECK:STDOUT:     %.loc13: type = converted %T.ref.loc13, %T.as_type.loc13_21.1 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.3df)]
+// CHECK:STDOUT:     %Interface.ref.loc13: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
+// CHECK:STDOUT:     %J.ref.loc13: type = name_ref J, file.%J.decl [concrete = constants.%J.type]
+// CHECK:STDOUT:     %T.loc13_14.1: %J.type = bind_symbolic_name T, 0 [symbolic = %T.loc13_14.2 (constants.%T.ccd)]
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %Interface.impl_witness_table.loc12 = impl_witness_table (), @impl.793 [concrete]
-// CHECK:STDOUT:   %Interface.impl_witness.loc12: <witness> = impl_witness %Interface.impl_witness_table.loc12, @impl.793(constants.%T.ccd) [symbolic = @impl.793.%Interface.impl_witness (constants.%Interface.impl_witness.608)]
+// CHECK:STDOUT:   %Interface.impl_witness_table.loc13 = impl_witness_table (), @impl.793 [concrete]
+// CHECK:STDOUT:   %Interface.impl_witness.loc13: <witness> = impl_witness %Interface.impl_witness_table.loc13, @impl.793(constants.%T.ccd) [symbolic = @impl.793.%Interface.impl_witness (constants.%Interface.impl_witness.608)]
 // CHECK:STDOUT:   impl_decl @impl.c93 [concrete] {
 // CHECK:STDOUT:     %T.patt: %pattern_type.4a6 = symbolic_binding_pattern T, 0 [concrete]
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %T.ref.loc13: %K.type = name_ref T, %T.loc13_14.1 [symbolic = %T.loc13_14.2 (constants.%T.09f)]
-// CHECK:STDOUT:     %T.as_type.loc13_21.1: type = facet_access_type %T.ref.loc13 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.037)]
-// CHECK:STDOUT:     %.loc13: type = converted %T.ref.loc13, %T.as_type.loc13_21.1 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.037)]
-// CHECK:STDOUT:     %Interface.ref.loc13: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
-// CHECK:STDOUT:     %K.ref.loc13: type = name_ref K, file.%K.decl [concrete = constants.%K.type]
-// CHECK:STDOUT:     %T.loc13_14.1: %K.type = bind_symbolic_name T, 0 [symbolic = %T.loc13_14.2 (constants.%T.09f)]
+// CHECK:STDOUT:     %T.ref.loc14: %K.type = name_ref T, %T.loc14_14.1 [symbolic = %T.loc14_14.2 (constants.%T.09f)]
+// CHECK:STDOUT:     %T.as_type.loc14_21.1: type = facet_access_type %T.ref.loc14 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.037)]
+// CHECK:STDOUT:     %.loc14: type = converted %T.ref.loc14, %T.as_type.loc14_21.1 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.037)]
+// CHECK:STDOUT:     %Interface.ref.loc14: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
+// CHECK:STDOUT:     %K.ref.loc14: type = name_ref K, file.%K.decl [concrete = constants.%K.type]
+// CHECK:STDOUT:     %T.loc14_14.1: %K.type = bind_symbolic_name T, 0 [symbolic = %T.loc14_14.2 (constants.%T.09f)]
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %Interface.impl_witness_table.loc13 = impl_witness_table (), @impl.c93 [concrete]
-// CHECK:STDOUT:   %Interface.impl_witness.loc13: <witness> = impl_witness %Interface.impl_witness_table.loc13, @impl.c93(constants.%T.09f) [symbolic = @impl.c93.%Interface.impl_witness (constants.%Interface.impl_witness.b9e)]
+// CHECK:STDOUT:   %Interface.impl_witness_table.loc14 = impl_witness_table (), @impl.c93 [concrete]
+// CHECK:STDOUT:   %Interface.impl_witness.loc14: <witness> = impl_witness %Interface.impl_witness_table.loc14, @impl.c93(constants.%T.09f) [symbolic = @impl.c93.%Interface.impl_witness (constants.%Interface.impl_witness.b9e)]
 // CHECK:STDOUT:   impl_decl @impl.9e6 [concrete] {
 // CHECK:STDOUT:     %T.patt: %pattern_type.86b = symbolic_binding_pattern T, 0 [concrete]
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %T.ref.loc14: %L.type = name_ref T, %T.loc14_14.1 [symbolic = %T.loc14_14.2 (constants.%T.1d2)]
-// CHECK:STDOUT:     %T.as_type.loc14_21.1: type = facet_access_type %T.ref.loc14 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.0ed)]
-// CHECK:STDOUT:     %.loc14: type = converted %T.ref.loc14, %T.as_type.loc14_21.1 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.0ed)]
-// CHECK:STDOUT:     %Interface.ref.loc14: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
-// CHECK:STDOUT:     %L.ref.loc14: type = name_ref L, file.%L.decl [concrete = constants.%L.type]
-// CHECK:STDOUT:     %T.loc14_14.1: %L.type = bind_symbolic_name T, 0 [symbolic = %T.loc14_14.2 (constants.%T.1d2)]
-// CHECK:STDOUT:   }
-// CHECK:STDOUT:   %Interface.impl_witness_table.loc14 = impl_witness_table (), @impl.9e6 [concrete]
-// CHECK:STDOUT:   %Interface.impl_witness.loc14: <witness> = impl_witness %Interface.impl_witness_table.loc14, @impl.9e6(constants.%T.1d2) [symbolic = @impl.9e6.%Interface.impl_witness (constants.%Interface.impl_witness.e9d)]
+// CHECK:STDOUT:     %T.ref.loc15: %L.type = name_ref T, %T.loc15_14.1 [symbolic = %T.loc15_14.2 (constants.%T.1d2)]
+// CHECK:STDOUT:     %T.as_type.loc15_21.1: type = facet_access_type %T.ref.loc15 [symbolic = %T.as_type.loc15_21.2 (constants.%T.as_type.0ed)]
+// CHECK:STDOUT:     %.loc15: type = converted %T.ref.loc15, %T.as_type.loc15_21.1 [symbolic = %T.as_type.loc15_21.2 (constants.%T.as_type.0ed)]
+// CHECK:STDOUT:     %Interface.ref.loc15: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
+// CHECK:STDOUT:     %L.ref.loc15: type = name_ref L, file.%L.decl [concrete = constants.%L.type]
+// CHECK:STDOUT:     %T.loc15_14.1: %L.type = bind_symbolic_name T, 0 [symbolic = %T.loc15_14.2 (constants.%T.1d2)]
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Interface.impl_witness_table.loc15 = impl_witness_table (), @impl.9e6 [concrete]
+// CHECK:STDOUT:   %Interface.impl_witness.loc15: <witness> = impl_witness %Interface.impl_witness_table.loc15, @impl.9e6(constants.%T.1d2) [symbolic = @impl.9e6.%Interface.impl_witness (constants.%Interface.impl_witness.e9d)]
 // CHECK:STDOUT:   impl_decl @impl.c3d [concrete] {
 // CHECK:STDOUT:     %T.patt: %pattern_type.2b5 = symbolic_binding_pattern T, 0 [concrete]
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %T.ref.loc18: %I.type = name_ref T, %T.loc18 [symbolic = %T.loc11_14.2 (constants.%T.826)]
-// CHECK:STDOUT:     %T.as_type.loc18: type = facet_access_type %T.ref.loc18 [symbolic = %T.as_type.loc11_21.2 (constants.%T.as_type.b70)]
-// CHECK:STDOUT:     %.loc18: type = converted %T.ref.loc18, %T.as_type.loc18 [symbolic = %T.as_type.loc11_21.2 (constants.%T.as_type.b70)]
-// CHECK:STDOUT:     %Interface.ref.loc18: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
-// CHECK:STDOUT:     %I.ref.loc18: type = name_ref I, file.%I.decl [concrete = constants.%I.type]
-// CHECK:STDOUT:     %T.loc18: %I.type = bind_symbolic_name T, 0 [symbolic = %T.loc11_14.2 (constants.%T.826)]
+// CHECK:STDOUT:     %T.ref.loc19: %I.type = name_ref T, %T.loc19 [symbolic = %T.loc12_14.2 (constants.%T.826)]
+// CHECK:STDOUT:     %T.as_type.loc19: type = facet_access_type %T.ref.loc19 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.b70)]
+// CHECK:STDOUT:     %.loc19: type = converted %T.ref.loc19, %T.as_type.loc19 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.b70)]
+// CHECK:STDOUT:     %Interface.ref.loc19: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
+// CHECK:STDOUT:     %I.ref.loc19: type = name_ref I, file.%I.decl [concrete = constants.%I.type]
+// CHECK:STDOUT:     %T.loc19: %I.type = bind_symbolic_name T, 0 [symbolic = %T.loc12_14.2 (constants.%T.826)]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   impl_decl @impl.793 [concrete] {
 // CHECK:STDOUT:     %T.patt: %pattern_type.28e = symbolic_binding_pattern T, 0 [concrete]
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %T.ref.loc26: %J.type = name_ref T, %T.loc26 [symbolic = %T.loc12_14.2 (constants.%T.ccd)]
-// CHECK:STDOUT:     %T.as_type.loc26: type = facet_access_type %T.ref.loc26 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.3df)]
-// CHECK:STDOUT:     %.loc26: type = converted %T.ref.loc26, %T.as_type.loc26 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.3df)]
-// CHECK:STDOUT:     %Interface.ref.loc26: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
-// CHECK:STDOUT:     %J.ref.loc26: type = name_ref J, file.%J.decl [concrete = constants.%J.type]
-// CHECK:STDOUT:     %T.loc26: %J.type = bind_symbolic_name T, 0 [symbolic = %T.loc12_14.2 (constants.%T.ccd)]
+// CHECK:STDOUT:     %T.ref.loc27: %J.type = name_ref T, %T.loc27 [symbolic = %T.loc13_14.2 (constants.%T.ccd)]
+// CHECK:STDOUT:     %T.as_type.loc27: type = facet_access_type %T.ref.loc27 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.3df)]
+// CHECK:STDOUT:     %.loc27: type = converted %T.ref.loc27, %T.as_type.loc27 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.3df)]
+// CHECK:STDOUT:     %Interface.ref.loc27: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
+// CHECK:STDOUT:     %J.ref.loc27: type = name_ref J, file.%J.decl [concrete = constants.%J.type]
+// CHECK:STDOUT:     %T.loc27: %J.type = bind_symbolic_name T, 0 [symbolic = %T.loc13_14.2 (constants.%T.ccd)]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   impl_decl @impl.c93 [concrete] {
 // CHECK:STDOUT:     %T.patt: %pattern_type.4a6 = symbolic_binding_pattern T, 0 [concrete]
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %T.ref.loc34: %K.type = name_ref T, %T.loc34 [symbolic = %T.loc13_14.2 (constants.%T.09f)]
-// CHECK:STDOUT:     %T.as_type.loc34: type = facet_access_type %T.ref.loc34 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.037)]
-// CHECK:STDOUT:     %.loc34: type = converted %T.ref.loc34, %T.as_type.loc34 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.037)]
-// CHECK:STDOUT:     %Interface.ref.loc34: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
-// CHECK:STDOUT:     %K.ref.loc34: type = name_ref K, file.%K.decl [concrete = constants.%K.type]
-// CHECK:STDOUT:     %T.loc34: %K.type = bind_symbolic_name T, 0 [symbolic = %T.loc13_14.2 (constants.%T.09f)]
+// CHECK:STDOUT:     %T.ref.loc35: %K.type = name_ref T, %T.loc35 [symbolic = %T.loc14_14.2 (constants.%T.09f)]
+// CHECK:STDOUT:     %T.as_type.loc35: type = facet_access_type %T.ref.loc35 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.037)]
+// CHECK:STDOUT:     %.loc35: type = converted %T.ref.loc35, %T.as_type.loc35 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.037)]
+// CHECK:STDOUT:     %Interface.ref.loc35: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
+// CHECK:STDOUT:     %K.ref.loc35: type = name_ref K, file.%K.decl [concrete = constants.%K.type]
+// CHECK:STDOUT:     %T.loc35: %K.type = bind_symbolic_name T, 0 [symbolic = %T.loc14_14.2 (constants.%T.09f)]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   impl_decl @impl.9e6 [concrete] {
 // CHECK:STDOUT:     %T.patt: %pattern_type.86b = symbolic_binding_pattern T, 0 [concrete]
 // CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %T.ref.loc42: %L.type = name_ref T, %T.loc42 [symbolic = %T.loc14_14.2 (constants.%T.1d2)]
-// CHECK:STDOUT:     %T.as_type.loc42: type = facet_access_type %T.ref.loc42 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.0ed)]
-// CHECK:STDOUT:     %.loc42: type = converted %T.ref.loc42, %T.as_type.loc42 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.0ed)]
-// CHECK:STDOUT:     %Interface.ref.loc42: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
-// CHECK:STDOUT:     %L.ref.loc42: type = name_ref L, file.%L.decl [concrete = constants.%L.type]
-// CHECK:STDOUT:     %T.loc42: %L.type = bind_symbolic_name T, 0 [symbolic = %T.loc14_14.2 (constants.%T.1d2)]
+// CHECK:STDOUT:     %T.ref.loc43: %L.type = name_ref T, %T.loc43 [symbolic = %T.loc15_14.2 (constants.%T.1d2)]
+// CHECK:STDOUT:     %T.as_type.loc43: type = facet_access_type %T.ref.loc43 [symbolic = %T.as_type.loc15_21.2 (constants.%T.as_type.0ed)]
+// CHECK:STDOUT:     %.loc43: type = converted %T.ref.loc43, %T.as_type.loc43 [symbolic = %T.as_type.loc15_21.2 (constants.%T.as_type.0ed)]
+// CHECK:STDOUT:     %Interface.ref.loc43: type = name_ref Interface, file.%Interface.decl [concrete = constants.%Interface.type]
+// CHECK:STDOUT:     %L.ref.loc43: type = name_ref L, file.%L.decl [concrete = constants.%L.type]
+// CHECK:STDOUT:     %T.loc43: %L.type = bind_symbolic_name T, 0 [symbolic = %T.loc15_14.2 (constants.%T.1d2)]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -298,79 +299,79 @@ impl forall [T:! type] T as I {
 // CHECK:STDOUT:   witness = ()
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: generic impl @impl.c3d(%T.loc11_14.1: %I.type) {
-// CHECK:STDOUT:   %T.loc11_14.2: %I.type = bind_symbolic_name T, 0 [symbolic = %T.loc11_14.2 (constants.%T.826)]
-// CHECK:STDOUT:   %T.as_type.loc11_21.2: type = facet_access_type %T.loc11_14.2 [symbolic = %T.as_type.loc11_21.2 (constants.%T.as_type.b70)]
-// CHECK:STDOUT:   %Interface.impl_witness: <witness> = impl_witness file.%Interface.impl_witness_table.loc11, @impl.c3d(%T.loc11_14.2) [symbolic = %Interface.impl_witness (constants.%Interface.impl_witness.a14)]
+// CHECK:STDOUT: generic impl @impl.c3d(%T.loc12_14.1: %I.type) {
+// CHECK:STDOUT:   %T.loc12_14.2: %I.type = bind_symbolic_name T, 0 [symbolic = %T.loc12_14.2 (constants.%T.826)]
+// CHECK:STDOUT:   %T.as_type.loc12_21.2: type = facet_access_type %T.loc12_14.2 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.b70)]
+// CHECK:STDOUT:   %Interface.impl_witness: <witness> = impl_witness file.%Interface.impl_witness_table.loc12, @impl.c3d(%T.loc12_14.2) [symbolic = %Interface.impl_witness (constants.%Interface.impl_witness.a14)]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !definition:
 // CHECK:STDOUT:
-// CHECK:STDOUT:   impl: %.loc11 as %Interface.ref.loc11 {
+// CHECK:STDOUT:   impl: %.loc12 as %Interface.ref.loc12 {
 // CHECK:STDOUT:   !members:
-// CHECK:STDOUT:     witness = file.%Interface.impl_witness.loc11
+// CHECK:STDOUT:     witness = file.%Interface.impl_witness.loc12
 // CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: generic impl @impl.793(%T.loc12_14.1: %J.type) {
-// CHECK:STDOUT:   %T.loc12_14.2: %J.type = bind_symbolic_name T, 0 [symbolic = %T.loc12_14.2 (constants.%T.ccd)]
-// CHECK:STDOUT:   %T.as_type.loc12_21.2: type = facet_access_type %T.loc12_14.2 [symbolic = %T.as_type.loc12_21.2 (constants.%T.as_type.3df)]
-// CHECK:STDOUT:   %Interface.impl_witness: <witness> = impl_witness file.%Interface.impl_witness_table.loc12, @impl.793(%T.loc12_14.2) [symbolic = %Interface.impl_witness (constants.%Interface.impl_witness.608)]
+// CHECK:STDOUT: generic impl @impl.793(%T.loc13_14.1: %J.type) {
+// CHECK:STDOUT:   %T.loc13_14.2: %J.type = bind_symbolic_name T, 0 [symbolic = %T.loc13_14.2 (constants.%T.ccd)]
+// CHECK:STDOUT:   %T.as_type.loc13_21.2: type = facet_access_type %T.loc13_14.2 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.3df)]
+// CHECK:STDOUT:   %Interface.impl_witness: <witness> = impl_witness file.%Interface.impl_witness_table.loc13, @impl.793(%T.loc13_14.2) [symbolic = %Interface.impl_witness (constants.%Interface.impl_witness.608)]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !definition:
 // CHECK:STDOUT:
-// CHECK:STDOUT:   impl: %.loc12 as %Interface.ref.loc12 {
+// CHECK:STDOUT:   impl: %.loc13 as %Interface.ref.loc13 {
 // CHECK:STDOUT:   !members:
-// CHECK:STDOUT:     witness = file.%Interface.impl_witness.loc12
+// CHECK:STDOUT:     witness = file.%Interface.impl_witness.loc13
 // CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: generic impl @impl.c93(%T.loc13_14.1: %K.type) {
-// CHECK:STDOUT:   %T.loc13_14.2: %K.type = bind_symbolic_name T, 0 [symbolic = %T.loc13_14.2 (constants.%T.09f)]
-// CHECK:STDOUT:   %T.as_type.loc13_21.2: type = facet_access_type %T.loc13_14.2 [symbolic = %T.as_type.loc13_21.2 (constants.%T.as_type.037)]
-// CHECK:STDOUT:   %Interface.impl_witness: <witness> = impl_witness file.%Interface.impl_witness_table.loc13, @impl.c93(%T.loc13_14.2) [symbolic = %Interface.impl_witness (constants.%Interface.impl_witness.b9e)]
+// CHECK:STDOUT: generic impl @impl.c93(%T.loc14_14.1: %K.type) {
+// CHECK:STDOUT:   %T.loc14_14.2: %K.type = bind_symbolic_name T, 0 [symbolic = %T.loc14_14.2 (constants.%T.09f)]
+// CHECK:STDOUT:   %T.as_type.loc14_21.2: type = facet_access_type %T.loc14_14.2 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.037)]
+// CHECK:STDOUT:   %Interface.impl_witness: <witness> = impl_witness file.%Interface.impl_witness_table.loc14, @impl.c93(%T.loc14_14.2) [symbolic = %Interface.impl_witness (constants.%Interface.impl_witness.b9e)]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !definition:
 // CHECK:STDOUT:
-// CHECK:STDOUT:   impl: %.loc13 as %Interface.ref.loc13 {
+// CHECK:STDOUT:   impl: %.loc14 as %Interface.ref.loc14 {
 // CHECK:STDOUT:   !members:
-// CHECK:STDOUT:     witness = file.%Interface.impl_witness.loc13
+// CHECK:STDOUT:     witness = file.%Interface.impl_witness.loc14
 // CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: generic impl @impl.9e6(%T.loc14_14.1: %L.type) {
-// CHECK:STDOUT:   %T.loc14_14.2: %L.type = bind_symbolic_name T, 0 [symbolic = %T.loc14_14.2 (constants.%T.1d2)]
-// CHECK:STDOUT:   %T.as_type.loc14_21.2: type = facet_access_type %T.loc14_14.2 [symbolic = %T.as_type.loc14_21.2 (constants.%T.as_type.0ed)]
-// CHECK:STDOUT:   %Interface.impl_witness: <witness> = impl_witness file.%Interface.impl_witness_table.loc14, @impl.9e6(%T.loc14_14.2) [symbolic = %Interface.impl_witness (constants.%Interface.impl_witness.e9d)]
+// CHECK:STDOUT: generic impl @impl.9e6(%T.loc15_14.1: %L.type) {
+// CHECK:STDOUT:   %T.loc15_14.2: %L.type = bind_symbolic_name T, 0 [symbolic = %T.loc15_14.2 (constants.%T.1d2)]
+// CHECK:STDOUT:   %T.as_type.loc15_21.2: type = facet_access_type %T.loc15_14.2 [symbolic = %T.as_type.loc15_21.2 (constants.%T.as_type.0ed)]
+// CHECK:STDOUT:   %Interface.impl_witness: <witness> = impl_witness file.%Interface.impl_witness_table.loc15, @impl.9e6(%T.loc15_14.2) [symbolic = %Interface.impl_witness (constants.%Interface.impl_witness.e9d)]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !definition:
 // CHECK:STDOUT:
-// CHECK:STDOUT:   impl: %.loc14 as %Interface.ref.loc14 {
+// CHECK:STDOUT:   impl: %.loc15 as %Interface.ref.loc15 {
 // CHECK:STDOUT:   !members:
-// CHECK:STDOUT:     witness = file.%Interface.impl_witness.loc14
+// CHECK:STDOUT:     witness = file.%Interface.impl_witness.loc15
 // CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @impl.c3d(constants.%T.826) {
-// CHECK:STDOUT:   %T.loc11_14.2 => constants.%T.826
-// CHECK:STDOUT:   %T.as_type.loc11_21.2 => constants.%T.as_type.b70
+// CHECK:STDOUT:   %T.loc12_14.2 => constants.%T.826
+// CHECK:STDOUT:   %T.as_type.loc12_21.2 => constants.%T.as_type.b70
 // CHECK:STDOUT:   %Interface.impl_witness => constants.%Interface.impl_witness.a14
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @impl.793(constants.%T.ccd) {
-// CHECK:STDOUT:   %T.loc12_14.2 => constants.%T.ccd
-// CHECK:STDOUT:   %T.as_type.loc12_21.2 => constants.%T.as_type.3df
+// CHECK:STDOUT:   %T.loc13_14.2 => constants.%T.ccd
+// CHECK:STDOUT:   %T.as_type.loc13_21.2 => constants.%T.as_type.3df
 // CHECK:STDOUT:   %Interface.impl_witness => constants.%Interface.impl_witness.608
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @impl.c93(constants.%T.09f) {
-// CHECK:STDOUT:   %T.loc13_14.2 => constants.%T.09f
-// CHECK:STDOUT:   %T.as_type.loc13_21.2 => constants.%T.as_type.037
+// CHECK:STDOUT:   %T.loc14_14.2 => constants.%T.09f
+// CHECK:STDOUT:   %T.as_type.loc14_21.2 => constants.%T.as_type.037
 // CHECK:STDOUT:   %Interface.impl_witness => constants.%Interface.impl_witness.b9e
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: specific @impl.9e6(constants.%T.1d2) {
-// CHECK:STDOUT:   %T.loc14_14.2 => constants.%T.1d2
-// CHECK:STDOUT:   %T.as_type.loc14_21.2 => constants.%T.as_type.0ed
+// CHECK:STDOUT:   %T.loc15_14.2 => constants.%T.1d2
+// CHECK:STDOUT:   %T.as_type.loc15_21.2 => constants.%T.as_type.0ed
 // CHECK:STDOUT:   %Interface.impl_witness => constants.%Interface.impl_witness.e9d
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

+ 4 - 2
toolchain/diagnostics/diagnostic_kind.def

@@ -316,8 +316,10 @@ CARBON_DIAGNOSTIC_KIND(ImplUnusedBinding)
 CARBON_DIAGNOSTIC_KIND(PoisonedImplLookupConcreteResult)
 CARBON_DIAGNOSTIC_KIND(PoisonedImplLookupConcreteResultNoteBadImpl)
 CARBON_DIAGNOSTIC_KIND(PoisonedImplLookupConcreteResultNotePreviousImpl)
-CARBON_DIAGNOSTIC_KIND(ImplFullyOverlapNonFinal)
-CARBON_DIAGNOSTIC_KIND(ImplFullyOverlapNonFinalNote)
+CARBON_DIAGNOSTIC_KIND(ImplFinalOverlapsNonFinal)
+CARBON_DIAGNOSTIC_KIND(ImplFinalOverlapsNonFinalNote)
+CARBON_DIAGNOSTIC_KIND(ImplNonFinalSameTypeStructure)
+CARBON_DIAGNOSTIC_KIND(ImplNonFinalSameTypeStructureNote)
 
 // Impl lookup.
 CARBON_DIAGNOSTIC_KIND(MissingImplInMemberAccess)