From 949ec17da226b513fd5544861c35d7dbe65236c2 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 25 Sep 2025 13:25:48 -0700 Subject: [PATCH] Improve interop for classes with multiple inheritance. (#6130) If there's a unique "preferred" base class, then treat that as "the" base class for Carbon's purposes. In particular: * If there's exactly one polymorphic base class, that's our preferred base class. * If there's exactly one non-empty base class, that's our perferred base class. * (Degenerate case) If there's exactly one base class, that's our preferred base class. --------- Co-authored-by: Jon Ross-Perkins --- toolchain/check/cpp/import.cpp | 36 ++++-- .../testdata/interop/cpp/class/base.carbon | 120 ++++++++++++++++++ 2 files changed, 147 insertions(+), 9 deletions(-) diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index 0c75f67a8895..ae482af8e6ed 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -828,6 +828,16 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id, // TODO: Import vptr(s). + // The kind of base class we've picked so far. These are ordered in increasing + // preference order. + enum class BaseKind { + None, + Empty, + NonEmpty, + Polymorphic, + }; + BaseKind base_kind = BaseKind::None; + // Import bases. for (const auto& base : clang_def->bases()) { CARBON_CHECK(!base.isVirtual(), @@ -849,19 +859,27 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id, .base_type_inst_id = base_type_inst_id, .index = SemIR::ElementIndex(fields.size())})); - // If there's exactly one base class, treat it as a Carbon base class too. - // TODO: Improve handling for the case where the class has multiple base - // classes. - if (clang_def->getNumBases() == 1) { - auto& class_info = context.classes().Get(class_id); - CARBON_CHECK(!class_info.base_id.has_value()); - class_info.base_id = base_decl_id; - } - auto* base_class = base.getType()->getAsCXXRecordDecl(); CARBON_CHECK(base_class, "Base class {0} is not a class", base.getType().getAsString()); + // If there's a unique "best" base class, treat it as a Carbon base class + // too. + // TODO: Improve handling for the case where the class has multiple base + // classes. + BaseKind kind = base_class->isPolymorphic() ? BaseKind::Polymorphic + : base_class->isEmpty() ? BaseKind::Empty + : BaseKind::NonEmpty; + auto& class_info = context.classes().Get(class_id); + if (kind > base_kind) { + // This base is better than the previous best. + class_info.base_id = base_decl_id; + base_kind = kind; + } else if (kind == base_kind) { + // Multiple base classes of this kind: no unique best. + class_info.base_id = SemIR::InstId::None; + } + auto base_offset = base.isVirtual() ? clang_layout.getVBaseClassOffset(base_class) : clang_layout.getBaseClassOffset(base_class); diff --git a/toolchain/check/testdata/interop/cpp/class/base.carbon b/toolchain/check/testdata/interop/cpp/class/base.carbon index 0d0ba8555da9..d44081defc6f 100644 --- a/toolchain/check/testdata/interop/cpp/class/base.carbon +++ b/toolchain/check/testdata/interop/cpp/class/base.carbon @@ -125,6 +125,20 @@ struct A { int a; }; struct B { int b; }; struct C : A, B {}; +struct Empty1 {}; +struct Empty2 {}; +struct OneNonEmptyBase : Empty1, A, Empty2 {}; +struct TwoNonEmptyBases : Empty1, A, B, Empty2 {}; + +struct Polymorphic1 { + virtual void f(); +}; +struct Polymorphic2 { + virtual void g(); +}; +struct OnePolymorphicBase : A, B, Polymorphic1, Empty1 {}; +struct TwoPolymorphicBases : Polymorphic1, Polymorphic2 {}; + // --- fail_todo_use_multiple_inheritance.carbon library "[[@TEST_NAME]]"; @@ -153,6 +167,112 @@ fn ConvertB(p: Cpp.C*) -> Cpp.B* { return p; } +fn ConvertOneNonEmptyBaseToEmpty1(p: Cpp.OneNonEmptyBase*) -> Cpp.Empty1* { + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.OneNonEmptyBase*` to `Cpp.Empty1*` [ConversionFailure] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.OneNonEmptyBase*` does not implement interface `Core.ImplicitAs(Cpp.Empty1*)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: + return p; +} + +fn ConvertOneNonEmptyBaseToEmpty2(p: Cpp.OneNonEmptyBase*) -> Cpp.Empty2* { + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.OneNonEmptyBase*` to `Cpp.Empty2*` [ConversionFailure] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.OneNonEmptyBase*` does not implement interface `Core.ImplicitAs(Cpp.Empty2*)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: + return p; +} + +fn ConvertTwoNonEmptyBasesToA(p: Cpp.TwoNonEmptyBases*) -> Cpp.A* { + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.TwoNonEmptyBases*` to `Cpp.A*` [ConversionFailure] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.TwoNonEmptyBases*` does not implement interface `Core.ImplicitAs(Cpp.A*)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: + return p; +} + +fn ConvertTwoNonEmptyBasesToB(p: Cpp.TwoNonEmptyBases*) -> Cpp.B* { + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.TwoNonEmptyBases*` to `Cpp.B*` [ConversionFailure] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.TwoNonEmptyBases*` does not implement interface `Core.ImplicitAs(Cpp.B*)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: + return p; +} + +fn ConvertOnePolymorphicBaseToA(p: Cpp.OnePolymorphicBase*) -> Cpp.A* { + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.OnePolymorphicBase*` to `Cpp.A*` [ConversionFailure] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.OnePolymorphicBase*` does not implement interface `Core.ImplicitAs(Cpp.A*)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: + return p; +} + +fn ConvertOnePolymorphicBaseToB(p: Cpp.OnePolymorphicBase*) -> Cpp.B* { + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.OnePolymorphicBase*` to `Cpp.B*` [ConversionFailure] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.OnePolymorphicBase*` does not implement interface `Core.ImplicitAs(Cpp.B*)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: + return p; +} + +fn ConvertTwoPolymorphicBasesToPolymorphic1(p: Cpp.TwoPolymorphicBases*) -> Cpp.Polymorphic1* { + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.TwoPolymorphicBases*` to `Cpp.Polymorphic1*` [ConversionFailure] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.TwoPolymorphicBases*` does not implement interface `Core.ImplicitAs(Cpp.Polymorphic1*)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: + return p; +} + +fn ConvertTwoPolymorphicBasesToPolymorphic2(p: Cpp.TwoPolymorphicBases*) -> Cpp.Polymorphic2* { + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.TwoPolymorphicBases*` to `Cpp.Polymorphic2*` [ConversionFailure] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.TwoPolymorphicBases*` does not implement interface `Core.ImplicitAs(Cpp.Polymorphic2*)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: return p; + // CHECK:STDERR: ^~~~~~~~~ + // CHECK:STDERR: + return p; +} + +// --- multiple_inheritance_with_preferred_base.carbon + +library "[[@TEST_NAME]]"; + +import Cpp library "multiple_inheritance.h"; + +// If there's only one non-empty base, that's our preferred base class. We can +// convert to that. +fn ConvertOneNonEmptyBaseToA(p: Cpp.OneNonEmptyBase*) -> Cpp.A* { + return p; +} + +// If there's only one polymorphic base, that's our preferred base class. We can +// convert to that. +fn ConvertOnePolymorphicBaseToPolymorphic1(p: Cpp.OnePolymorphicBase*) -> Cpp.Polymorphic1* { + return p; +} + // --- virtual_inheritance.h struct A { int a; };