Skip to content

Commit 949ec17

Browse files
zygoloidjonmeow
zygoloid
and
jonmeow
authored
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 <[email protected]>
1 parent f194acb commit 949ec17

File tree

2 files changed

+146
-8
lines changed

2 files changed

+146
-8
lines changed

‎toolchain/check/cpp/import.cpp‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,16 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id,
828828

829829
// TODO: Import vptr(s).
830830

831+
// The kind of base class we've picked so far. These are ordered in increasing
832+
// preference order.
833+
enum class BaseKind {
834+
None,
835+
Empty,
836+
NonEmpty,
837+
Polymorphic,
838+
};
839+
BaseKind base_kind = BaseKind::None;
840+
831841
// Import bases.
832842
for (const auto& base : clang_def->bases()) {
833843
CARBON_CHECK(!base.isVirtual(),
@@ -849,19 +859,27 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id,
849859
.base_type_inst_id = base_type_inst_id,
850860
.index = SemIR::ElementIndex(fields.size())}));
851861

852-
// If there's exactly one base class, treat it as a Carbon base class too.
862+
auto* base_class = base.getType()->getAsCXXRecordDecl();
863+
CARBON_CHECK(base_class, "Base class {0} is not a class",
864+
base.getType().getAsString());
865+
866+
// If there's a unique "best" base class, treat it as a Carbon base class
867+
// too.
853868
// TODO: Improve handling for the case where the class has multiple base
854869
// classes.
855-
if (clang_def->getNumBases() == 1) {
856-
auto& class_info = context.classes().Get(class_id);
857-
CARBON_CHECK(!class_info.base_id.has_value());
870+
BaseKind kind = base_class->isPolymorphic() ? BaseKind::Polymorphic
871+
: base_class->isEmpty() ? BaseKind::Empty
872+
: BaseKind::NonEmpty;
873+
auto& class_info = context.classes().Get(class_id);
874+
if (kind > base_kind) {
875+
// This base is better than the previous best.
858876
class_info.base_id = base_decl_id;
877+
base_kind = kind;
878+
} else if (kind == base_kind) {
879+
// Multiple base classes of this kind: no unique best.
880+
class_info.base_id = SemIR::InstId::None;
859881
}
860882

861-
auto* base_class = base.getType()->getAsCXXRecordDecl();
862-
CARBON_CHECK(base_class, "Base class {0} is not a class",
863-
base.getType().getAsString());
864-
865883
auto base_offset = base.isVirtual()
866884
? clang_layout.getVBaseClassOffset(base_class)
867885
: clang_layout.getBaseClassOffset(base_class);

‎toolchain/check/testdata/interop/cpp/class/base.carbon‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,20 @@ struct A { int a; };
125125
struct B { int b; };
126126
struct C : A, B {};
127127

128+
struct Empty1 {};
129+
struct Empty2 {};
130+
struct OneNonEmptyBase : Empty1, A, Empty2 {};
131+
struct TwoNonEmptyBases : Empty1, A, B, Empty2 {};
132+
133+
struct Polymorphic1 {
134+
virtual void f();
135+
};
136+
struct Polymorphic2 {
137+
virtual void g();
138+
};
139+
struct OnePolymorphicBase : A, B, Polymorphic1, Empty1 {};
140+
struct TwoPolymorphicBases : Polymorphic1, Polymorphic2 {};
141+
128142
// --- fail_todo_use_multiple_inheritance.carbon
129143

130144
library "[[@TEST_NAME]]";
@@ -153,6 +167,112 @@ fn ConvertB(p: Cpp.C*) -> Cpp.B* {
153167
return p;
154168
}
155169

170+
fn ConvertOneNonEmptyBaseToEmpty1(p: Cpp.OneNonEmptyBase*) -> Cpp.Empty1* {
171+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.OneNonEmptyBase*` to `Cpp.Empty1*` [ConversionFailure]
172+
// CHECK:STDERR: return p;
173+
// CHECK:STDERR: ^~~~~~~~~
174+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.OneNonEmptyBase*` does not implement interface `Core.ImplicitAs(Cpp.Empty1*)` [MissingImplInMemberAccessNote]
175+
// CHECK:STDERR: return p;
176+
// CHECK:STDERR: ^~~~~~~~~
177+
// CHECK:STDERR:
178+
return p;
179+
}
180+
181+
fn ConvertOneNonEmptyBaseToEmpty2(p: Cpp.OneNonEmptyBase*) -> Cpp.Empty2* {
182+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.OneNonEmptyBase*` to `Cpp.Empty2*` [ConversionFailure]
183+
// CHECK:STDERR: return p;
184+
// CHECK:STDERR: ^~~~~~~~~
185+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.OneNonEmptyBase*` does not implement interface `Core.ImplicitAs(Cpp.Empty2*)` [MissingImplInMemberAccessNote]
186+
// CHECK:STDERR: return p;
187+
// CHECK:STDERR: ^~~~~~~~~
188+
// CHECK:STDERR:
189+
return p;
190+
}
191+
192+
fn ConvertTwoNonEmptyBasesToA(p: Cpp.TwoNonEmptyBases*) -> Cpp.A* {
193+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.TwoNonEmptyBases*` to `Cpp.A*` [ConversionFailure]
194+
// CHECK:STDERR: return p;
195+
// CHECK:STDERR: ^~~~~~~~~
196+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.TwoNonEmptyBases*` does not implement interface `Core.ImplicitAs(Cpp.A*)` [MissingImplInMemberAccessNote]
197+
// CHECK:STDERR: return p;
198+
// CHECK:STDERR: ^~~~~~~~~
199+
// CHECK:STDERR:
200+
return p;
201+
}
202+
203+
fn ConvertTwoNonEmptyBasesToB(p: Cpp.TwoNonEmptyBases*) -> Cpp.B* {
204+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.TwoNonEmptyBases*` to `Cpp.B*` [ConversionFailure]
205+
// CHECK:STDERR: return p;
206+
// CHECK:STDERR: ^~~~~~~~~
207+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.TwoNonEmptyBases*` does not implement interface `Core.ImplicitAs(Cpp.B*)` [MissingImplInMemberAccessNote]
208+
// CHECK:STDERR: return p;
209+
// CHECK:STDERR: ^~~~~~~~~
210+
// CHECK:STDERR:
211+
return p;
212+
}
213+
214+
fn ConvertOnePolymorphicBaseToA(p: Cpp.OnePolymorphicBase*) -> Cpp.A* {
215+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.OnePolymorphicBase*` to `Cpp.A*` [ConversionFailure]
216+
// CHECK:STDERR: return p;
217+
// CHECK:STDERR: ^~~~~~~~~
218+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.OnePolymorphicBase*` does not implement interface `Core.ImplicitAs(Cpp.A*)` [MissingImplInMemberAccessNote]
219+
// CHECK:STDERR: return p;
220+
// CHECK:STDERR: ^~~~~~~~~
221+
// CHECK:STDERR:
222+
return p;
223+
}
224+
225+
fn ConvertOnePolymorphicBaseToB(p: Cpp.OnePolymorphicBase*) -> Cpp.B* {
226+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.OnePolymorphicBase*` to `Cpp.B*` [ConversionFailure]
227+
// CHECK:STDERR: return p;
228+
// CHECK:STDERR: ^~~~~~~~~
229+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.OnePolymorphicBase*` does not implement interface `Core.ImplicitAs(Cpp.B*)` [MissingImplInMemberAccessNote]
230+
// CHECK:STDERR: return p;
231+
// CHECK:STDERR: ^~~~~~~~~
232+
// CHECK:STDERR:
233+
return p;
234+
}
235+
236+
fn ConvertTwoPolymorphicBasesToPolymorphic1(p: Cpp.TwoPolymorphicBases*) -> Cpp.Polymorphic1* {
237+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.TwoPolymorphicBases*` to `Cpp.Polymorphic1*` [ConversionFailure]
238+
// CHECK:STDERR: return p;
239+
// CHECK:STDERR: ^~~~~~~~~
240+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.TwoPolymorphicBases*` does not implement interface `Core.ImplicitAs(Cpp.Polymorphic1*)` [MissingImplInMemberAccessNote]
241+
// CHECK:STDERR: return p;
242+
// CHECK:STDERR: ^~~~~~~~~
243+
// CHECK:STDERR:
244+
return p;
245+
}
246+
247+
fn ConvertTwoPolymorphicBasesToPolymorphic2(p: Cpp.TwoPolymorphicBases*) -> Cpp.Polymorphic2* {
248+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.TwoPolymorphicBases*` to `Cpp.Polymorphic2*` [ConversionFailure]
249+
// CHECK:STDERR: return p;
250+
// CHECK:STDERR: ^~~~~~~~~
251+
// CHECK:STDERR: fail_todo_use_multiple_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.TwoPolymorphicBases*` does not implement interface `Core.ImplicitAs(Cpp.Polymorphic2*)` [MissingImplInMemberAccessNote]
252+
// CHECK:STDERR: return p;
253+
// CHECK:STDERR: ^~~~~~~~~
254+
// CHECK:STDERR:
255+
return p;
256+
}
257+
258+
// --- multiple_inheritance_with_preferred_base.carbon
259+
260+
library "[[@TEST_NAME]]";
261+
262+
import Cpp library "multiple_inheritance.h";
263+
264+
// If there's only one non-empty base, that's our preferred base class. We can
265+
// convert to that.
266+
fn ConvertOneNonEmptyBaseToA(p: Cpp.OneNonEmptyBase*) -> Cpp.A* {
267+
return p;
268+
}
269+
270+
// If there's only one polymorphic base, that's our preferred base class. We can
271+
// convert to that.
272+
fn ConvertOnePolymorphicBaseToPolymorphic1(p: Cpp.OnePolymorphicBase*) -> Cpp.Polymorphic1* {
273+
return p;
274+
}
275+
156276
// --- virtual_inheritance.h
157277

158278
struct A { int a; };

0 commit comments

Comments
 (0)