Skip to content

Commit 0d0be26

Browse files
zoecarverzoecarver
authored
Merge pull request #61535 from zoecarver/frt-with-base-class
[cxx-interop] Fix issue with layout of FRTs that contain base classes or un-importable members.
2 parents bec9e54 + c5aaa60 commit 0d0be26

File tree

5 files changed

+187
-2
lines changed

5 files changed

+187
-2
lines changed

‎lib/IRGen/GenClass.cpp‎

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
#include "GenClass.h"
1818

19+
#include "clang/AST/ASTContext.h"
20+
#include "clang/AST/Decl.h"
21+
#include "clang/AST/RecordLayout.h"
1922
#include "swift/ABI/Class.h"
2023
#include "swift/ABI/MetadataValues.h"
2124
#include "swift/AST/ASTContext.h"
@@ -28,6 +31,7 @@
2831
#include "swift/AST/SemanticAttrs.h"
2932
#include "swift/AST/TypeMemberVisitor.h"
3033
#include "swift/AST/Types.h"
34+
#include "swift/Basic/Defer.h"
3135
#include "swift/ClangImporter/ClangModule.h"
3236
#include "swift/IRGen/Linking.h"
3337
#include "swift/SIL/SILModule.h"
@@ -281,6 +285,55 @@ namespace {
281285
superclass);
282286
}
283287

288+
void maybeAddCxxRecordBases(ClassDecl *cd) {
289+
auto cxxRecord = dyn_cast_or_null<clang::CXXRecordDecl>(cd->getClangDecl());
290+
if (!cxxRecord)
291+
return;
292+
293+
auto &layout = cxxRecord->getASTContext().getASTRecordLayout(cxxRecord);
294+
295+
for (auto base : cxxRecord->bases()) {
296+
if (base.isVirtual())
297+
continue;
298+
299+
auto baseType = base.getType().getCanonicalType();
300+
301+
auto baseRecord = cast<clang::RecordType>(baseType)->getDecl();
302+
auto baseCxxRecord = cast<clang::CXXRecordDecl>(baseRecord);
303+
304+
if (baseCxxRecord->isEmpty())
305+
continue;
306+
307+
auto offset = Size(layout.getBaseClassOffset(baseCxxRecord).getQuantity());
308+
auto size = Size(cxxRecord->getASTContext().getTypeSizeInChars(baseType).getQuantity());
309+
310+
if (offset != CurSize) {
311+
assert(offset > CurSize);
312+
auto paddingSize = offset - CurSize;
313+
auto &opaqueTI = IGM.getOpaqueStorageTypeInfo(paddingSize, Alignment(1));
314+
auto element = ElementLayout::getIncomplete(opaqueTI);
315+
addField(element, LayoutStrategy::Universal);
316+
}
317+
318+
auto &opaqueTI = IGM.getOpaqueStorageTypeInfo(size, Alignment(1));
319+
auto element = ElementLayout::getIncomplete(opaqueTI);
320+
addField(element, LayoutStrategy::Universal);
321+
}
322+
}
323+
324+
void addPaddingBeforeClangField(const clang::FieldDecl *fd) {
325+
auto offset = Size(fd->getASTContext().toCharUnitsFromBits(
326+
fd->getASTContext().getFieldOffset(fd)).getQuantity());
327+
328+
if (offset != CurSize) {
329+
assert(offset > CurSize);
330+
auto paddingSize = offset - CurSize;
331+
auto &opaqueTI = IGM.getOpaqueStorageTypeInfo(paddingSize, Alignment(1));
332+
auto element = ElementLayout::getIncomplete(opaqueTI);
333+
addField(element, LayoutStrategy::Universal);
334+
}
335+
}
336+
284337
void addDirectFieldsFromClass(ClassDecl *rootClass, SILType rootClassType,
285338
ClassDecl *theClass, SILType classType,
286339
bool superclass) {
@@ -290,7 +343,9 @@ namespace {
290343
->getRecursiveProperties()
291344
.hasUnboundGeneric());
292345

293-
forEachField(IGM, theClass, [&](Field field) {
346+
maybeAddCxxRecordBases(theClass);
347+
348+
auto fn = [&](Field field) {
294349
// Ignore missing properties here; we should have flagged these
295350
// with the classHasIncompleteLayout call above.
296351
if (!field.isConcrete()) {
@@ -325,7 +380,25 @@ namespace {
325380
AllStoredProperties.push_back(field);
326381
AllFieldAccesses.push_back(getFieldAccess(isKnownEmpty));
327382
}
328-
});
383+
};
384+
385+
auto classDecl = dyn_cast<ClassDecl>(theClass);
386+
if (classDecl && classDecl->isRootDefaultActor()) {
387+
fn(Field::DefaultActorStorage);
388+
}
389+
390+
for (auto decl :
391+
theClass->getStoredPropertiesAndMissingMemberPlaceholders()) {
392+
if (decl->getClangDecl())
393+
if (auto clangField = cast<clang::FieldDecl>(decl->getClangDecl()))
394+
addPaddingBeforeClangField(clangField);
395+
396+
if (auto var = dyn_cast<VarDecl>(decl)) {
397+
fn(var);
398+
} else {
399+
fn(cast<MissingMemberDecl>(decl));
400+
}
401+
}
329402

330403
if (!superclass) {
331404
// If we're calculating the layout of a specialized generic class type,

‎test/Interop/Cxx/foreign-reference/Inputs/member-layout.h‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
#ifndef TEST_INTEROP_CXX_FOREIGN_REFERENCE_INPUTS_NULLABLE_H
2+
#define TEST_INTEROP_CXX_FOREIGN_REFERENCE_INPUTS_NULLABLE_H
3+
4+
struct IntHolder { int value; };
5+
6+
struct
7+
__attribute__((swift_attr("import_reference")))
8+
__attribute__((swift_attr("retain:immortal")))
9+
__attribute__((swift_attr("release:immortal")))
10+
IntBase : IntHolder {
11+
int i;
12+
};
13+
14+
struct NoDtorThreeByte {
15+
char x;
16+
char y;
17+
char z;
18+
~NoDtorThreeByte() = delete;
19+
};
20+
21+
struct
22+
__attribute__((swift_attr("import_reference")))
23+
__attribute__((swift_attr("retain:immortal")))
24+
__attribute__((swift_attr("release:immortal")))
25+
IntCharRef {
26+
int i;
27+
char b;
28+
};
29+
30+
struct
31+
__attribute__((swift_attr("import_reference")))
32+
__attribute__((swift_attr("retain:immortal")))
33+
__attribute__((swift_attr("release:immortal")))
34+
IntCharValue {
35+
int i;
36+
char b;
37+
};
38+
39+
40+
struct
41+
__attribute__((swift_attr("import_reference")))
42+
__attribute__((swift_attr("retain:immortal")))
43+
__attribute__((swift_attr("release:immortal")))
44+
UnimportableMemberRef {
45+
int z; int zz; NoDtorThreeByte x; NoDtorThreeByte xx; int y;
46+
};
47+
48+
struct
49+
__attribute__((swift_attr("import_reference")))
50+
__attribute__((swift_attr("retain:immortal")))
51+
__attribute__((swift_attr("release:immortal")))
52+
UnimportableMemberValue {
53+
int z; int zz; NoDtorThreeByte x; NoDtorThreeByte xx; int y;
54+
};
55+
56+
#endif // TEST_INTEROP_CXX_FOREIGN_REFERENCE_INPUTS_NULLABLE_H

‎test/Interop/Cxx/foreign-reference/Inputs/module.modulemap‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,8 @@ module ReferenceCounted {
2727
header "reference-counted.h"
2828
requires cplusplus
2929
}
30+
31+
module MemberLayout {
32+
header "member-layout.h"
33+
requires cplusplus
34+
}

‎test/Interop/Cxx/foreign-reference/base-class-layout-irgen.swift‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: %target-swift-emit-ir %s -I %S/Inputs -enable-experimental-cxx-interop -module-name=test | %FileCheck %s
2+
//
3+
// XFAIL: OS=linux-android, OS=linux-androideabi
4+
5+
import MemberLayout
6+
7+
// CHECK: %TSo7IntBaseV = type <{ [4 x i8], %Ts5Int32V }>
8+
9+
// CHECK-LABEL: define {{.*}}swiftcc i32 @"$s4testAA1ys5Int32VSo7IntBaseV_tF"(%TSo7IntBaseV* %0)
10+
// CHECK: getelementptr inbounds %TSo7IntBaseV, %TSo7IntBaseV* %0, i32 0, i32 1
11+
public func test(y: IntBase) -> CInt {
12+
return y.i
13+
}

‎test/Interop/Cxx/foreign-reference/unimportable-member-layout-irgen.swift‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// RUN: %target-swift-emit-ir %s -I %S/Inputs -enable-experimental-cxx-interop -module-name=test | %FileCheck %s
2+
//
3+
// XFAIL: OS=linux-android, OS=linux-androideabi
4+
5+
import MemberLayout
6+
7+
// Make sure the "refs" and "values" look *exactly* the same.
8+
9+
// CHECK: %TSo10IntCharRefV = type <{ %Ts5Int32V, %Ts4Int8V }>
10+
// CHECK: %TSo12IntCharValueV = type <{ %Ts5Int32V, %Ts4Int8V }>
11+
12+
// CHECK: %TSo21UnimportableMemberRefV = type <{ %Ts5Int32V, %Ts5Int32V, [8 x i8], %Ts5Int32V }>
13+
// CHECK: %TSo23UnimportableMemberValueV = type <{ %Ts5Int32V, %Ts5Int32V, [8 x i8], %Ts5Int32V }>
14+
15+
// CHECK-LABEL: define {{.*}}swiftcc i8 @"$s4testAA1ys4Int8VSo10IntCharRefV_tF"(%TSo10IntCharRefV* %0)
16+
// CHECK: getelementptr inbounds %TSo10IntCharRefV, %TSo10IntCharRefV* %0, i32 0, i32 1
17+
public func test(y: IntCharRef) -> CChar {
18+
return y.b
19+
}
20+
21+
// CHECK-LABEL: define {{.*}}swiftcc i8 @"$s4testAA1ys4Int8VSo12IntCharValueV_tF"(%TSo12IntCharValueV* %0)
22+
// CHECK: getelementptr inbounds %TSo12IntCharValueV, %TSo12IntCharValueV* %0, i32 0, i32 1
23+
public func test(y: IntCharValue) -> CChar {
24+
return y.b
25+
}
26+
27+
// CHECK-LABEL: define {{.*}}swiftcc i32 @"$s4testAA1ys5Int32VSo21UnimportableMemberRefV_tF"(%TSo21UnimportableMemberRefV* %0)
28+
// CHECK: getelementptr inbounds %TSo21UnimportableMemberRefV, %TSo21UnimportableMemberRefV* %0, i32 0, i32 3
29+
public func test(y: UnimportableMemberRef) -> CInt {
30+
return y.y
31+
}
32+
33+
// CHECK-LABEL: define {{.*}}swiftcc i32 @"$s4testAA1ys5Int32VSo23UnimportableMemberValueV_tF"(%TSo23UnimportableMemberValueV* %0)
34+
// CHECK: getelementptr inbounds %TSo23UnimportableMemberValueV, %TSo23UnimportableMemberValueV* %0, i32 0, i32 3
35+
public func test(y: UnimportableMemberValue) -> CInt {
36+
return y.y
37+
}
38+

0 commit comments

Comments
 (0)