Przeglądaj źródła

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 <jperkins@google.com>
Richard Smith 7 miesięcy temu
rodzic
commit
949ec17da2

+ 26 - 8
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.
+    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.
-    if (clang_def->getNumBases() == 1) {
-      auto& class_info = context.classes().Get(class_id);
-      CARBON_CHECK(!class_info.base_id.has_value());
+    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_class = base.getType()->getAsCXXRecordDecl();
-    CARBON_CHECK(base_class, "Base class {0} is not a class",
-                 base.getType().getAsString());
-
     auto base_offset = base.isVirtual()
                            ? clang_layout.getVBaseClassOffset(base_class)
                            : clang_layout.getBaseClassOffset(base_class);

+ 120 - 0
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; };