diff --git a/toolchain/check/check_unit.cpp b/toolchain/check/check_unit.cpp index d363f4f5bb25..2567abed29b8 100644 --- a/toolchain/check/check_unit.cpp +++ b/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(witness_id) - .witness_table_id; - return context_.insts() - .GetAs(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(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(); diff --git a/toolchain/check/testdata/impl/lookup/fail_poison_custom_witness.carbon b/toolchain/check/testdata/impl/lookup/fail_poison_custom_witness.carbon new file mode 100644 index 000000000000..bd0a6e8296e8 --- /dev/null +++ b/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; +'''; diff --git a/toolchain/diagnostics/kind.def b/toolchain/diagnostics/kind.def index 3ffbb8faa4b9..c01c1c39db7b 100644 --- a/toolchain/diagnostics/kind.def +++ 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)