From 07111398d880dc24f280f250f52e9d50fd80e7c2 Mon Sep 17 00:00:00 2001 From: arhwx Date: Mon, 27 Jul 2026 10:49:06 -0400 Subject: [PATCH] Anchor the orphan rule on the first owning declaration (#7573) `DiagnoseOrphanImpl` used `definition_id`, but #7140 defines the anchor as the first owning declaration, so a class declared but not defined was rejected, which is why three cases in `orphan.carbon` were marked `fail_todo`, and they now pass. `fail_use_extern_class` no longer errors, `handle_class.cpp` never passes the `extern library` name into the class entity, so `C` is treated as locally owned and counts as an anchor. The expected error is replaced with a TODO in the test, but it should come back once `extern library` is implemented for classes. --- toolchain/check/impl_validation.cpp | 16 +++++++----- toolchain/check/testdata/impl/orphan.carbon | 28 +++++---------------- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/toolchain/check/impl_validation.cpp b/toolchain/check/impl_validation.cpp index 6d5c0b881ec9..bf2d7def3449 100644 --- a/toolchain/check/impl_validation.cpp +++ b/toolchain/check/impl_validation.cpp @@ -169,14 +169,16 @@ static auto DiagnoseOrphanImpl(Context& context, const ImplInfo& impl, using Step = SemIR::TypeIterator::Step; CARBON_KIND_SWITCH(step.any) { case CARBON_KIND(Step::ClassStart start): { - auto inst_id = context.classes().Get(start.class_id).definition_id; + auto inst_id = + context.classes().Get(start.class_id).first_owning_decl_id; if (IsSameLibrary(context, inst_id)) { return true; } break; } case CARBON_KIND(Step::ClassStartOnly start): { - auto inst_id = context.classes().Get(start.class_id).definition_id; + auto inst_id = + context.classes().Get(start.class_id).first_owning_decl_id; if (IsSameLibrary(context, inst_id)) { return true; } @@ -191,7 +193,7 @@ static auto DiagnoseOrphanImpl(Context& context, const ImplInfo& impl, CARBON_KIND_SWITCH(context.types().GetAsInst(type.type_id)) { case CARBON_KIND(SemIR::GenericClassType class_type): { auto class_id = class_type.class_id; - auto inst_id = context.classes().Get(class_id).definition_id; + auto inst_id = context.classes().Get(class_id).first_owning_decl_id; if (IsSameLibrary(context, inst_id)) { return true; } @@ -199,7 +201,8 @@ static auto DiagnoseOrphanImpl(Context& context, const ImplInfo& impl, } case CARBON_KIND(SemIR::GenericInterfaceType interface_type): { auto interface_id = interface_type.interface_id; - auto inst_id = context.interfaces().Get(interface_id).definition_id; + auto inst_id = + context.interfaces().Get(interface_id).first_owning_decl_id; if (IsSameLibrary(context, inst_id)) { return true; } @@ -207,8 +210,9 @@ static auto DiagnoseOrphanImpl(Context& context, const ImplInfo& impl, } case CARBON_KIND(SemIR::GenericNamedConstraintType constraint_type): { auto constraint_id = constraint_type.named_constraint_id; - auto inst_id = - context.named_constraints().Get(constraint_id).definition_id; + auto inst_id = context.named_constraints() + .Get(constraint_id) + .first_owning_decl_id; if (IsSameLibrary(context, inst_id)) { return true; } diff --git a/toolchain/check/testdata/impl/orphan.carbon b/toolchain/check/testdata/impl/orphan.carbon index 264f32a92553..6cb9e9309868 100644 --- a/toolchain/check/testdata/impl/orphan.carbon +++ b/toolchain/check/testdata/impl/orphan.carbon @@ -23,7 +23,7 @@ constraint ImportedN(T: type) {} class ImportedAnyParam[T: type](X: T) {} -// --- fail_todo_class_definition_missing.carbon +// --- class_definition_missing.carbon library "[[@TEST_NAME]]"; import library "imports"; @@ -31,14 +31,9 @@ import library "imports"; // This is the first owning decl of C, so it can be an anchor name. class C; -// TODO: This should be accepted. -// CHECK:STDERR: fail_todo_class_definition_missing.carbon:[[@LINE+4]]:1: error: orphan `impl` found; something in the self-type or constraint must be defined in the same file [ImplIsOrphan] -// CHECK:STDERR: impl C as ImportedY {} -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: impl C as ImportedY {} -// --- fail_todo_class_definition_missing_for_self_specific.carbon +// --- class_definition_missing_for_self_specific.carbon library "[[@TEST_NAME]]"; import library "imports"; @@ -46,14 +41,9 @@ import library "imports"; // This is the first owning decl of C, so it can be an anchor name. class C; -// TODO: This should be accepted. -// CHECK:STDERR: fail_todo_class_definition_missing_for_self_specific.carbon:[[@LINE+4]]:1: error: orphan `impl` found; something in the self-type or constraint must be defined in the same file [ImplIsOrphan] -// CHECK:STDERR: impl ImportedD(C) as ImportedY {} -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: impl ImportedD(C) as ImportedY {} -// --- fail_todo_class_definition_missing_for_interface_specific.carbon +// --- class_definition_missing_for_interface_specific.carbon library "[[@TEST_NAME]]"; import library "imports"; @@ -61,11 +51,6 @@ import library "imports"; // This is the first owning decl of C, so it can be an anchor name. class C; -// TODO: This should be accepted. -// CHECK:STDERR: fail_todo_class_definition_missing_for_interface_specific.carbon:[[@LINE+4]]:1: error: orphan `impl` found; something in the self-type or constraint must be defined in the same file [ImplIsOrphan] -// CHECK:STDERR: impl ImportedC as ImportedZ(C) {} -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: impl ImportedC as ImportedZ(C) {} // --- fail_imported_class.carbon @@ -402,10 +387,9 @@ import library "imports"; // CHECK:STDERR: extern library "extern_class" class C; -// CHECK:STDERR: fail_use_extern_class.carbon:[[@LINE+4]]:1: error: orphan `impl` found; something in the self-type or constraint must be defined in the same file [ImplIsOrphan] -// CHECK:STDERR: impl C as ImportedY {} -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: +// TODO: This should be an orphan `impl`. `C` is owned by `extern_class`, so it +// can't be an anchor name. No error will be emitted until `extern library` is +// implemented for classes. impl C as ImportedY {} // --- imported_first_owning_decl.carbon