From db24042fe56d22275aa801696e2f8f5c4171e35b Mon Sep 17 00:00:00 2001 From: Dana Jansens Date: Wed, 6 May 2026 10:17:54 -0400 Subject: [PATCH] Diagnose overlapping impls in the api/impl files of the same library (#7164) An `impl` decl in an impl file can refer only to things defined in the api file, without the orphan rule rejecting it. If they are in different scopes (such as one being in a class and one not), then they are treated as separate `impl` decls. But if they have the same type structure, then they fully overlap which is an error unless they are in a match_first block. This catches the overlap when two `impl` decls are in the same library but are split between the api and the impl file of the library. Previously we only diagnosed if they were in the same _file_ but now we diagnose if they are in the same _library_. --- toolchain/check/impl_validation.cpp | 13 ++++++++++--- .../check/testdata/impl/redeclaration.carbon | 16 +++++++++++++--- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/toolchain/check/impl_validation.cpp b/toolchain/check/impl_validation.cpp index e57a816ac59e..6b8b307396ec 100644 --- a/toolchain/check/impl_validation.cpp +++ b/toolchain/check/impl_validation.cpp @@ -35,6 +35,7 @@ struct ImplInfo { bool is_local; // If imported, the IR from which the `impl` decl was imported. SemIR::ImportIRId ir_id; + SemIR::LibraryNameId library_id; std::optional type_structure; }; @@ -80,6 +81,9 @@ static auto IsSameLibrary(Context& context, SemIR::InstId owning_inst_id) static auto GetImplInfo(Context& context, SemIR::ImplId impl_id) -> ImplInfo { const auto& impl = context.impls().Get(impl_id); auto ir_id = GetIRId(context, impl.first_owning_decl_id); + auto library_id = ir_id.has_value() + ? context.import_irs().Get(ir_id).sem_ir->library_id() + : context.sem_ir().library_id(); return {.impl_id = impl_id, .is_final = impl.is_final, .witness_id = impl.witness_id, @@ -88,6 +92,7 @@ static auto GetImplInfo(Context& context, SemIR::ImplId impl_id) -> ImplInfo { .interface = impl.interface, .is_local = !ir_id.has_value(), .ir_id = ir_id, + .library_id = library_id, .type_structure = BuildTypeStructure(context, impl.self_id, impl.interface)}; } @@ -406,10 +411,12 @@ static auto ValidateImplsForInterface(Context& context, // Rules between two non-final impls. // ===================================================================== if (!did_diagnose_non_final_impls_with_same_type_structure) { - // Two impls in separate files will need to have some different + // Two impls in separate libraries will need to have some different // concrete element in their type structure, as enforced by the orphan - // rule. So we don't need to check against non-local impls. - if (impl_a.is_local && impl_b.is_local) { + // rule. So we only need to check when they are in the same library, + // but possibly different files, if one is in the api and one in the + // impl file. + if (impl_a.library_id == impl_b.library_id) { if (DiagnoseNonFinalImplsWithSameTypeStructure(context, impl_a, impl_b)) { // The same final `impl_a` may overlap with multiple `impl_b`s, diff --git a/toolchain/check/testdata/impl/redeclaration.carbon b/toolchain/check/testdata/impl/redeclaration.carbon index 4a07f3ea6a5a..1539baf53e9d 100644 --- a/toolchain/check/testdata/impl/redeclaration.carbon +++ b/toolchain/check/testdata/impl/redeclaration.carbon @@ -219,11 +219,21 @@ interface Z {} class C {} impl C as Z {} -// --- todo_fail_decl_of_imported_impl_in_new_scope.impl.carbon +// --- fail_decl_of_imported_impl_in_new_scope.impl.carbon impl library "[[@TEST_NAME]]"; class D { - // TODO: This is not treated as a redeclaration since it is in a different - // scope. But it should be rejected by the orphan rule v2. + // This is not treated as a redeclaration since it is in a different scope. + // But that means it overlaps fully with the impl from the api file, which is + // a separate error. + + // CHECK:STDERR: fail_decl_of_imported_impl_in_new_scope.impl.carbon:[[@LINE+8]]:3: error: found non-final `impl` with the same type structure as another non-final `impl` [ImplNonFinalSameTypeStructure] + // CHECK:STDERR: impl C as Z {} + // CHECK:STDERR: ^~~~~~~~~~~~~ + // CHECK:STDERR: fail_decl_of_imported_impl_in_new_scope.impl.carbon:[[@LINE-10]]:1: in import [InImport] + // CHECK:STDERR: decl_of_imported_impl_in_new_scope.carbon:5:1: note: other `impl` here [ImplNonFinalSameTypeStructureNote] + // CHECK:STDERR: impl C as Z {} + // CHECK:STDERR: ^~~~~~~~~~~~~ + // CHECK:STDERR: impl C as Z {} }