Skip to content

Commit f6f5adb

Browse files
committed
[cxx-interop] Avoid querying layout of dependent types during symbolic import
In swiftlang#80786, we started importing certain padded fields as opaque blobs. Part of this logic involved querying those fields' ASTRecordLayout. However, dependent types (which are imported symbolically) do not have an ASTRecordLayout, so calling Clang's getASTRecordLayout() would lead to an assertion error for class templates where a no_unique_address field is some kind of dependent C++ record type. This patch avoids the field padding check during symbolic import mode because that check is only relevant for codegen anyway. rdar://150067288 (cherry picked from commit 1fdb239)
1 parent a2c5cff commit f6f5adb

File tree

3 files changed

+35
-2
lines changed

3 files changed

+35
-2
lines changed

lib/ClangImporter/ImportDecl.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4472,11 +4472,17 @@ namespace {
44724472
}
44734473

44744474
Decl *VisitFieldDecl(const clang::FieldDecl *decl) {
4475-
if (decl->hasAttr<clang::NoUniqueAddressAttr>()) {
4475+
if (!Impl.importSymbolicCXXDecls &&
4476+
decl->hasAttr<clang::NoUniqueAddressAttr>()) {
44764477
if (const auto *rd = decl->getType()->getAsRecordDecl()) {
44774478
// Clang can store the next field in the padding of this one. Swift
44784479
// does not support this yet so let's not import the field and
44794480
// represent it with an opaque blob in codegen.
4481+
//
4482+
// This check is not relevant when importing the decl symbolically
4483+
// (since that isn't used for codegen). In fact, we need to avoid this
4484+
// check because symbolic imports can expose us to dependent types
4485+
// whose ASTRecordLayout cannot be queried.
44804486
const auto &fieldLayout =
44814487
decl->getASTContext().getASTRecordLayout(rd);
44824488
auto &clangCtx = decl->getASTContext();

test/Interop/Cxx/class/Inputs/member-variables.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,18 @@ struct ReuseNonStandardLayoutFieldPadding {
6767
int offset() const { return (char *) &this->c - (char *) this; }
6868
};
6969

70+
template<typename T>
71+
struct ReuseDependentFieldPadding {
72+
[[no_unique_address]] struct { private: T x; public: char pad_me; } a;
73+
char c;
74+
char get_c() const { return c; }
75+
void set_c(char c) { this->c = c; }
76+
// C-style implementation of offsetof() to avoid non-standard-layout warning
77+
int offset() const { return (char *) &this->c - (char *) this; }
78+
};
79+
80+
typedef ReuseDependentFieldPadding<int> ReuseDependentFieldPaddingInt;
81+
7082
inline int takesZeroSizedInCpp(HasZeroSizedField x) {
7183
return x.a;
7284
}

test/Interop/Cxx/class/zero-sized-field.swift

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop -Xcc -std=c++20)
1+
// RUN: %empty-directory(%t/index)
2+
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop -Xcc -std=c++20 -Xfrontend -index-store-path -Xfrontend %t/index)
23
//
34
// REQUIRES: executable_test
45

@@ -68,4 +69,18 @@ FieldsTestSuite.test("Non-standard-layout field padding reused") {
6869
expectEqual(s2.get_c(), 6)
6970
}
7071

72+
FieldsTestSuite.test("Non-standard-layout field padding in templated class reused") {
73+
var s = ReuseDependentFieldPaddingInt()
74+
s.c = 5
75+
expectEqual(Int(s.offset()), MemoryLayout<ReuseDependentFieldPaddingInt>.offset(of: \.c)!)
76+
expectEqual(s.c, 5)
77+
expectEqual(s.get_c(), 5)
78+
s.set_c(6)
79+
expectEqual(s.c, 6)
80+
expectEqual(s.get_c(), 6)
81+
let s2 = s
82+
expectEqual(s2.c, 6)
83+
expectEqual(s2.get_c(), 6)
84+
}
85+
7186
runAllTests()

0 commit comments

Comments
 (0)