Skip to content

Allow interop with classes with virtual base classes. #6413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 21, 2025
+321 −67
Merged

Allow interop with classes with virtual base classes. #6413

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d7c516
Allow interop with classes with virtual base classes.
zygoloid Nov 20, 2025
e79eb73
Try to make comment clearer to people who aren't me.
zygoloid Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions toolchain/check/cpp/import.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,13 @@ static auto GetInheritanceKind(clang::CXXRecordDecl* class_def)
return SemIR::Class::Final;
}

if (class_def->getNumVBases()) {
// TODO: We treat classes with virtual bases as final for now. We use the
// layout of the class including its virtual bases as its Carbon type
// layout, so we wouldn't behave correctly if we derived from it.
return SemIR::Class::Final;
}

if (class_def->isAbstract()) {
// If the class has any abstract members, it's abstract.
return SemIR::Class::Abstract;
Expand Down Expand Up @@ -802,8 +809,15 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id,

// Import bases.
for (const auto& base : clang_def->bases()) {
CARBON_CHECK(!base.isVirtual(),
"Should not import definition for class with a virtual base");
if (base.isVirtual()) {
// If the base is virtual, skip it from the layout. We don't know where it
// will actually appear within the complete object layout, as a pointer to
// this class might point to a derived type that puts the vbase in a
// different place.
// TODO: Track that the virtual base existed. Support derived-to-vbase
// conversions by generating a clang AST fragment.
continue;
}

auto [base_type_inst_id, base_type_id] =
ImportTypeAndDependencies(context, import_ir_inst_id, base.getType());
Expand Down Expand Up @@ -842,6 +856,10 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id,
class_info.base_id = SemIR::InstId::None;
}

// TODO: If the base class has virtual bases, the size of the type that we
// add to the layout here will be the full size of the class (including
// virtual bases), whereas the size actually occupied by this base class is
// only the nvsize (excluding virtual bases).
auto base_offset = base.isVirtual()
? clang_layout.getVBaseClassOffset(base_class)
: clang_layout.getBaseClassOffset(base_class);
Expand Down Expand Up @@ -2413,17 +2431,6 @@ auto ImportClassDefinitionForClangDecl(Context& context, SemIR::LocId loc_id,
auto* class_def = class_decl->getDefinition();
CARBON_CHECK(class_def, "Complete type has no definition");

if (class_def->getNumVBases()) {
// TODO: Handle virtual bases. We don't actually know where they go in the
// layout. We may also want to use a different size in the layout for
// `partial C`, excluding the virtual base. It's also not entirely safe to
// just skip over the virtual base, as the type we would construct would
// have a misleading size. For now, treat a C++ class with vbases as
// incomplete in Carbon.
context.TODO(loc_id, "class with virtual bases");
return false;
}

BuildClassDefinition(context, import_ir_inst_id, class_id, class_inst_id,
class_def);
} else if (auto* enum_decl = dyn_cast<clang::EnumDecl>(clang_decl)) {
Expand Down
110 changes: 104 additions & 6 deletions toolchain/check/testdata/interop/cpp/class/base.carbon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,30 +298,128 @@ library "[[@TEST_NAME]]";
import Cpp library "virtual_inheritance.h";

fn Convert(p: Cpp.B*) -> Cpp.A* {
// CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+21]]:3: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
// CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.B*` to `Cpp.A*` [ConversionFailure]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+18]]:3: note: while completing C++ type `Cpp.B` [InCppTypeCompletion]
// CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.B*` does not implement interface `Core.ImplicitAs(Cpp.A*)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR:
// CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+14]]:3: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
return p;
}

// --- diamond.h

struct A {
int a;
};

struct B : virtual A {
int b;
};

struct C : virtual A {
int c;
};

struct D : B, C {
int d;
};

// --- fail_todo_use_diamond.carbon

library "[[@TEST_NAME]]";

import Cpp library "diamond.h";

fn ConvertBA(p: Cpp.B*) -> Cpp.A* {
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.B*` to `Cpp.A*` [ConversionFailure]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+11]]:3: note: while completing C++ type `Cpp.B` [InCppTypeCompletion]
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+4]]:3: note: type `Cpp.B*` does not implement interface `Core.ImplicitAs(Cpp.A*)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR:
// CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.B*` to `Cpp.A*` [ConversionFailure]
return p;
}

fn ConvertCA(p: Cpp.C*) -> Cpp.A* {
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.C*` to `Cpp.A*` [ConversionFailure]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+4]]:3: note: type `Cpp.B*` does not implement interface `Core.ImplicitAs(Cpp.A*)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+4]]:3: note: type `Cpp.C*` does not implement interface `Core.ImplicitAs(Cpp.A*)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR:
return p;
}

fn ConvertDB(p: Cpp.D*) -> Cpp.B* {
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.D*` to `Cpp.B*` [ConversionFailure]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+4]]:3: note: type `Cpp.D*` does not implement interface `Core.ImplicitAs(Cpp.B*)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR:
return p;
}

fn ConvertDC(p: Cpp.D*) -> Cpp.C* {
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+7]]:3: error: cannot implicitly convert expression of type `Cpp.D*` to `Cpp.C*` [ConversionFailure]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+4]]:3: note: type `Cpp.D*` does not implement interface `Core.ImplicitAs(Cpp.C*)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: return p;
// CHECK:STDERR: ^~~~~~~~~
// CHECK:STDERR:
return p;
}

fn AccessBA(d: Cpp.D) -> i32 {
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+7]]:11: error: cannot convert expression of type `Cpp.D` to `Cpp.B` with `as` [ConversionFailure]
// CHECK:STDERR: return (d as Cpp.B).b;
// CHECK:STDERR: ^~~~~~~~~~
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+4]]:11: note: type `Cpp.D` does not implement interface `Core.As(Cpp.B)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: return (d as Cpp.B).b;
// CHECK:STDERR: ^~~~~~~~~~
// CHECK:STDERR:
return (d as Cpp.B).b;
}

fn AccessCA(d: Cpp.D) -> i32 {
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+7]]:11: error: cannot convert expression of type `Cpp.D` to `Cpp.C` with `as` [ConversionFailure]
// CHECK:STDERR: return (d as Cpp.C).b;
// CHECK:STDERR: ^~~~~~~~~~
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+4]]:11: note: type `Cpp.D` does not implement interface `Core.As(Cpp.C)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: return (d as Cpp.C).b;
// CHECK:STDERR: ^~~~~~~~~~
// CHECK:STDERR:
return (d as Cpp.C).b;
}

fn AccessDB(d: Cpp.D) -> i32 {
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+7]]:10: error: cannot implicitly convert expression of type `Cpp.D` to `Cpp.B` [ConversionFailure]
// CHECK:STDERR: return d.b;
// CHECK:STDERR: ^~~
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+4]]:10: note: type `Cpp.D` does not implement interface `Core.ImplicitAs(Cpp.B)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: return d.b;
// CHECK:STDERR: ^~~
// CHECK:STDERR:
return d.b;
}

fn AccessDC(d: Cpp.D) -> i32 {
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+7]]:10: error: cannot implicitly convert expression of type `Cpp.D` to `Cpp.C` [ConversionFailure]
// CHECK:STDERR: return d.c;
// CHECK:STDERR: ^~~
// CHECK:STDERR: fail_todo_use_diamond.carbon:[[@LINE+4]]:10: note: type `Cpp.D` does not implement interface `Core.ImplicitAs(Cpp.C)` [MissingImplInMemberAccessNote]
// CHECK:STDERR: return d.c;
// CHECK:STDERR: ^~~
// CHECK:STDERR:
return d.c;
}

// --- final.h

struct A final {};
Expand Down
48 changes: 0 additions & 48 deletions toolchain/check/testdata/interop/cpp/function/operators.carbon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,54 +877,6 @@ fn F() {
let result: i32 = *CreateIncomplete() + complete;
}

// ============================================================================
// Unsupported operand type in instantiation
// ============================================================================

// --- unsupported_in_instantiation.h

struct Supported {};

template<typename T>
struct Unsupported : public virtual Supported {};
using UnsupportedAlias = Unsupported<int>;
extern UnsupportedAlias unsupported;

// --- fail_import_unsupported_in_instantiation_unary.carbon

library "[[@TEST_NAME]]";

import Cpp library "unsupported_in_instantiation.h";

fn F() {
// CHECK:STDERR: fail_import_unsupported_in_instantiation_unary.carbon:[[@LINE+7]]:21: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
// CHECK:STDERR: let result: i32 = -Cpp.unsupported;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~
// CHECK:STDERR: fail_import_unsupported_in_instantiation_unary.carbon:[[@LINE+4]]:21: note: while completing C++ type `Cpp.Unsupported` [InCppTypeCompletion]
// CHECK:STDERR: let result: i32 = -Cpp.unsupported;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~
// CHECK:STDERR:
let result: i32 = -Cpp.unsupported;
}

// --- fail_import_unsupported_in_instantiation_binary.carbon

library "[[@TEST_NAME]]";

import Cpp library "unsupported_in_instantiation.h";

fn F() {
var supported: Cpp.Supported = Cpp.Supported.Supported();
// CHECK:STDERR: fail_import_unsupported_in_instantiation_binary.carbon:[[@LINE+7]]:21: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
// CHECK:STDERR: let result: i32 = supported + Cpp.unsupported;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR: fail_import_unsupported_in_instantiation_binary.carbon:[[@LINE+4]]:21: note: while completing C++ type `Cpp.Unsupported` [InCppTypeCompletion]
// CHECK:STDERR: let result: i32 = supported + Cpp.unsupported;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR:
let result: i32 = supported + Cpp.unsupported;
}

// ============================================================================
// Indirect template instantiation
// ============================================================================
Expand Down
Loading
Loading