Skip to content

Commit 68869c6

Browse files
authored
Merge pull request #80786 from swiftlang/gaborh/unique-address-padding
2 parents 30a72ab + 939ee49 commit 68869c6

File tree

4 files changed

+89
-2
lines changed

4 files changed

+89
-2
lines changed

lib/ClangImporter/ImportDecl.cpp

+23
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
#include "clang/AST/DeclCXX.h"
6262
#include "clang/AST/DeclObjCCommon.h"
6363
#include "clang/AST/PrettyPrinter.h"
64+
#include "clang/AST/RecordLayout.h"
6465
#include "clang/AST/Type.h"
6566
#include "clang/Basic/Specifiers.h"
6667
#include "clang/Basic/TargetInfo.h"
@@ -4479,6 +4480,28 @@ namespace {
44794480
}
44804481

44814482
Decl *VisitFieldDecl(const clang::FieldDecl *decl) {
4483+
if (decl->hasAttr<clang::NoUniqueAddressAttr>()) {
4484+
if (const auto *rd = decl->getType()->getAsRecordDecl()) {
4485+
// Clang can store the next field in the padding of this one. Swift
4486+
// does not support this yet so let's not import the field and
4487+
// represent it with an opaque blob in codegen.
4488+
const auto &fieldLayout =
4489+
decl->getASTContext().getASTRecordLayout(rd);
4490+
auto &clangCtx = decl->getASTContext();
4491+
if (!decl->isZeroSize(clangCtx) &&
4492+
fieldLayout.getDataSize() != fieldLayout.getSize()) {
4493+
const auto *parent = decl->getParent();
4494+
auto currIdx = decl->getFieldIndex();
4495+
auto nextIdx = currIdx + 1;
4496+
const auto &parentLayout = clangCtx.getASTRecordLayout(parent);
4497+
if (parentLayout.getFieldCount() > nextIdx &&
4498+
parentLayout.getFieldOffset(nextIdx) <
4499+
(parentLayout.getFieldOffset(currIdx) +
4500+
clangCtx.toBits(fieldLayout.getSize())))
4501+
return nullptr;
4502+
}
4503+
}
4504+
}
44824505
// Fields are imported as variables.
44834506
std::optional<ImportedName> correctSwiftName;
44844507
ImportedName importedName;

lib/IRGen/GenStruct.cpp

+12-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "GenStruct.h"
1818

19+
#include "IRGen.h"
1920
#include "swift/AST/ClangModuleLoader.h"
2021
#include "swift/AST/ConformanceLookup.h"
2122
#include "swift/AST/Decl.h"
@@ -42,6 +43,7 @@
4243
#include "llvm/ADT/STLExtras.h"
4344
#include "llvm/IR/DerivedTypes.h"
4445
#include "llvm/IR/Function.h"
46+
#include "llvm/Support/Error.h"
4547
#include <iterator>
4648

4749
#include "GenDecl.h"
@@ -1459,6 +1461,14 @@ class ClangRecordLowering {
14591461
unsigned fieldOffset = layout.getFieldOffset(clangField->getFieldIndex());
14601462
assert(!clangField->isBitField());
14611463
Size offset( SubobjectAdjustment.getValue() + fieldOffset / 8);
1464+
std::optional<Size> dataSize;
1465+
if (clangField->hasAttr<clang::NoUniqueAddressAttr>()) {
1466+
if (const auto *rd = clangField->getType()->getAsRecordDecl()) {
1467+
// Clang can store the next field in the padding of this one.
1468+
const auto &fieldLayout = ClangContext.getASTRecordLayout(rd);
1469+
dataSize = Size(fieldLayout.getDataSize().getQuantity());
1470+
}
1471+
}
14621472

14631473
// If we have a Swift import of this type, use our lowered information.
14641474
if (swiftField) {
@@ -1471,7 +1481,8 @@ class ClangRecordLowering {
14711481

14721482
// Otherwise, add it as an opaque blob.
14731483
auto fieldSize = isZeroSized ? clang::CharUnits::Zero() : ClangContext.getTypeSizeInChars(clangField->getType());
1474-
return addOpaqueField(offset, Size(fieldSize.getQuantity()));
1484+
return addOpaqueField(offset,
1485+
dataSize.value_or(Size(fieldSize.getQuantity())));
14751486
}
14761487

14771488
/// Add opaque storage for bitfields spanning the given range of bits.

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

+23
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
#ifndef TEST_INTEROP_CXX_CLASS_INPUTS_MEMBER_VARIABLES_H
22
#define TEST_INTEROP_CXX_CLASS_INPUTS_MEMBER_VARIABLES_H
33

4+
#include <cstddef>
5+
#include <optional>
6+
47
class MyClass {
58
public:
69
const int const_member = 23;
@@ -24,6 +27,26 @@ struct HasZeroSizedField {
2427
void set_c(short c) { this->c = c; }
2528
};
2629

30+
struct ReuseFieldPadding {
31+
[[no_unique_address]] std::optional<int> a = {2};
32+
char c;
33+
char get_c() const { return c; }
34+
void set_c(char c) { this->c = c; }
35+
int offset() const { return offsetof(ReuseFieldPadding, c); }
36+
std::optional<int> getOptional() { return a; }
37+
};
38+
39+
using OptInt = std::optional<int>;
40+
41+
struct ReuseFieldPaddingWithTypedef {
42+
[[no_unique_address]] OptInt a;
43+
char c;
44+
char get_c() const { return c; }
45+
void set_c(char c) { this->c = c; }
46+
int offset() const { return offsetof(ReuseFieldPadding, c); }
47+
};
48+
49+
2750
inline int takesZeroSizedInCpp(HasZeroSizedField x) {
2851
return x.a;
2952
}

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

+31-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ FieldsTestSuite.test("Zero sized field") {
1515
s.set_c(7)
1616
takeTypeWithZeroSizedMember(s)
1717
let s2 = s
18-
let myInt : Empty.type = 6
18+
let _ : Empty.type = 6
1919
expectEqual(s.a, 5)
2020
expectEqual(s.a, s.get_a())
2121
expectEqual(s2.c, 7)
@@ -24,4 +24,34 @@ FieldsTestSuite.test("Zero sized field") {
2424
expectEqual(s.b.getNum(), 42)
2525
}
2626

27+
FieldsTestSuite.test("Field padding reused") {
28+
var s = ReuseFieldPadding()
29+
let opt = s.getOptional()
30+
expectEqual(Int(opt.pointee), 2)
31+
s.c = 5
32+
expectEqual(Int(s.offset()), MemoryLayout<ReuseFieldPadding>.offset(of: \.c)!)
33+
expectEqual(s.c, 5)
34+
expectEqual(s.get_c(), 5)
35+
s.set_c(6)
36+
expectEqual(s.c, 6)
37+
expectEqual(s.get_c(), 6)
38+
let s2 = s
39+
expectEqual(s2.c, 6)
40+
expectEqual(s2.get_c(), 6)
41+
}
42+
43+
FieldsTestSuite.test("Typedef'd field padding reused") {
44+
var s = ReuseFieldPaddingWithTypedef()
45+
s.c = 5
46+
expectEqual(Int(s.offset()), MemoryLayout<ReuseFieldPadding>.offset(of: \.c)!)
47+
expectEqual(s.c, 5)
48+
expectEqual(s.get_c(), 5)
49+
s.set_c(6)
50+
expectEqual(s.c, 6)
51+
expectEqual(s.get_c(), 6)
52+
let s2 = s
53+
expectEqual(s2.c, 6)
54+
expectEqual(s2.get_c(), 6)
55+
}
56+
2757
runAllTests()

0 commit comments

Comments
 (0)