Diagnose when an impl is added for something that has a custom witness (#7623)

The toolchain crash is easily diagnosed, but looking at the log doesn't
offer immediate insight into why the program crashes. This provides us
with a graceful exit.

toolchain/check/check_unit.cpp

@@ -536,41 +536,68 @@ auto CheckUnit::CheckPoisonedConcreteImplLookupQueries() -> void {
     }
     if (found_witness_id != poison.witness_id) {
       auto witness_to_impl_id = [&](SemIR::ConstantId witness_id) {
-        auto table_id = context_.constant_values()
-                            .GetInstAs<SemIR::ImplWitness>(witness_id)
-                            .witness_table_id;
-        return context_.insts()
-            .GetAs<SemIR::ImplWitnessTable>(table_id)
-            .impl_id;
+        auto inst = context_.constant_values().GetInst(witness_id);
+        CARBON_KIND_SWITCH(inst) {
+          case CARBON_KIND(SemIR::ImplWitness impl_witness): {
+            auto table_id = impl_witness.witness_table_id;
+            return context_.insts()
+                .GetAs<SemIR::ImplWitnessTable>(table_id)
+                .impl_id;
+          }
+          case CARBON_KIND(SemIR::CustomWitness _): {
+            return SemIR::ImplId::None;
+          }
+          default:
+            CARBON_FATAL("Unsupported entity");
+        }
       };
 
       // We can get the `Impl` from the resulting witness here, which is the
       // `Impl` that conflicts with the previous poison query.
       auto bad_impl_id = witness_to_impl_id(found_witness_id);
-      const auto& bad_impl = context_.impls().Get(bad_impl_id);
-
       auto prev_impl_id = witness_to_impl_id(poison.witness_id);
+      CARBON_CHECK(prev_impl_id.has_value(),
+                   "previous implementation should always have a value");
       const auto& prev_impl = context_.impls().Get(prev_impl_id);
+      if (bad_impl_id.has_value()) {
+        const auto& bad_impl = context_.impls().Get(bad_impl_id);
+        CARBON_DIAGNOSTIC(
+            PoisonedImplLookupConcreteResult, Error,
+            "found `impl` that would change the result of an earlier "
+            "use of `{0} as {1}`",
+            InstIdAsRawType, SpecificInterfaceIdAsRawType);
+        CARBON_DIAGNOSTIC(
+            PoisonedImplLookupConcreteResultNoteBadImpl, Note,
+            "the use would select the `impl` here but it was not found yet");
+        CARBON_DIAGNOSTIC(PoisonedImplLookupConcreteResultNotePreviousImpl,
+                          Note, "the use had selected the `impl` here");
+        emitter_
+            .Build(poison.loc_id, PoisonedImplLookupConcreteResult,
+                   poison.query.query_self_inst_id,
+                   poison.query.query_specific_interface_id)
+            .Note(bad_impl.first_decl_id(),
+                  PoisonedImplLookupConcreteResultNoteBadImpl)
+            .Note(prev_impl.first_decl_id(),
+                  PoisonedImplLookupConcreteResultNotePreviousImpl)
+            .Emit();
+      } else {
+        CARBON_DIAGNOSTIC(
+            PoisonedImplLookupCustomResult, Error,
+            "found `impl` for {0} as {1}, which has a custom witness",
+            InstIdAsRawType, SpecificInterfaceIdAsRawType);
+        CARBON_DIAGNOSTIC(
+            PoisonedImplLookupCustomResultNoteBadImpl, Note,
+            "the use tried to select the `impl` here, but a custom "
+            "witness was already chosen");
 
-      CARBON_DIAGNOSTIC(
-          PoisonedImplLookupConcreteResult, Error,
-          "found `impl` that would change the result of an earlier "
-          "use of `{0} as {1}`",
-          InstIdAsRawType, SpecificInterfaceIdAsRawType);
-      auto builder =
-          emitter_.Build(poison.loc_id, PoisonedImplLookupConcreteResult,
-                         poison.query.query_self_inst_id,
-                         poison.query.query_specific_interface_id);
-      CARBON_DIAGNOSTIC(
-          PoisonedImplLookupConcreteResultNoteBadImpl, Note,
-          "the use would select the `impl` here but it was not found yet");
-      builder.Note(bad_impl.first_decl_id(),
-                   PoisonedImplLookupConcreteResultNoteBadImpl);
-      CARBON_DIAGNOSTIC(PoisonedImplLookupConcreteResultNotePreviousImpl, Note,
-                        "the use had selected the `impl` here");
-      builder.Note(prev_impl.first_decl_id(),
-                   PoisonedImplLookupConcreteResultNotePreviousImpl);
-      builder.Emit();
+        emitter_
+            .Build(poison.loc_id, PoisonedImplLookupCustomResult,
+                   poison.query.query_self_inst_id,
+                   poison.query.query_specific_interface_id)
+            .Note(prev_impl.first_decl_id(),
+                  PoisonedImplLookupCustomResultNoteBadImpl)
+            .Emit();
+      }
     }
   }
   context_.inst_block_stack().PopAndDiscard();

toolchain/check/testdata/impl/lookup/fail_poison_custom_witness.carbon

@@ -0,0 +1,54 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/none.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/lookup/fail_poison_custom_witness.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/lookup/fail_poison_custom_witness.carbon
+
+// --- destroy.carbon
+
+package Core library "destroy";
+
+interface Destroy {
+  fn Op(ref self);
+}
+
+impl forall [T: type] T as Destroy {
+  fn Op(unused ref self) {}
+}
+
+// --- fail_poison_custom_witness.carbon
+
+library "[[@TEST_NAME]]";
+import Core library "destroy";
+import Cpp;
+
+inline Cpp '''
+struct X {};
+
+struct A {
+  virtual void f(const X& x) = 0;
+};
+''';
+
+// CHECK:STDERR: fail_poison_custom_witness.carbon:[[@LINE+8]]:1: error: found `impl` for Cpp.A as Destroy, which has a custom witness [PoisonedImplLookupCustomResult]
+// CHECK:STDERR: class B {
+// CHECK:STDERR: ^~~~~~~~~
+// CHECK:STDERR: fail_poison_custom_witness.carbon:[[@LINE-14]]:1: in import [InImport]
+// CHECK:STDERR: destroy.carbon:8:1: note: the use tried to select the `impl` here, but a custom witness was already chosen [PoisonedImplLookupCustomResultNoteBadImpl]
+// CHECK:STDERR: impl forall [T: type] T as Destroy {
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+class B {
+  extend base: Cpp.A;
+  override fn f(unused self, unused x: Cpp.X) {}
+}
+
+inline Cpp '''
+Carbon::B b;
+''';

toolchain/diagnostics/kind.def

@@ -387,6 +387,8 @@ CARBON_DIAGNOSTIC_KIND(ImplRedefinition)
 CARBON_DIAGNOSTIC_KIND(ImplOfUnidentifiedFacetType)
 CARBON_DIAGNOSTIC_KIND(ImplOfNotOneInterface)
 CARBON_DIAGNOSTIC_KIND(ImplUnusedBinding)
+CARBON_DIAGNOSTIC_KIND(PoisonedImplLookupCustomResult)
+CARBON_DIAGNOSTIC_KIND(PoisonedImplLookupCustomResultNoteBadImpl)
 CARBON_DIAGNOSTIC_KIND(PoisonedImplLookupConcreteResult)
 CARBON_DIAGNOSTIC_KIND(PoisonedImplLookupConcreteResultNoteBadImpl)
 CARBON_DIAGNOSTIC_KIND(PoisonedImplLookupConcreteResultNotePreviousImpl)