Skip to content

Commit abd2549

Browse files
susmonteirosusmonteiro
committed
[cxx-interop] Fix metadata mismatch regarding fields of structs
1 parent cfe6195 commit abd2549

11 files changed

+265
-28
lines changed

‎lib/IRGen/Field.h

+10
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,22 @@ struct Field {
9696
bool operator!=(Field other) const { return declOrKind != other.declOrKind; }
9797
};
9898

99+
bool isPrivateField(VarDecl *field);
100+
101+
// Don't export private C++ fields that were imported as private Swift fields.
102+
// The type of a private field might not have all the type witness operations
103+
// that Swift requires, for instance, `std::unique_ptr<IncompleteType>` would
104+
// not have a destructor.
105+
bool isExportableField(Field field);
106+
99107
/// Iterate all the fields of the given struct or class type, including
100108
/// any implicit fields that might be accounted for in
101109
/// getFieldVectorLength.
102110
void forEachField(IRGenModule &IGM, const NominalTypeDecl *typeDecl,
103111
llvm::function_ref<void(Field field)> fn);
104112

113+
unsigned countExportableFields(IRGenModule &IGM, const NominalTypeDecl *type);
114+
105115
} // end namespace irgen
106116
} // end namespace swift
107117

‎lib/IRGen/GenMeta.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1840,7 +1840,7 @@ namespace {
18401840

18411841
void addLayoutInfo() {
18421842
// uint32_t NumFields;
1843-
B.addInt32(getNumFields(getType()));
1843+
B.addInt32(countExportableFields(IGM, getType()));
18441844

18451845
// uint32_t FieldOffsetVectorOffset;
18461846
B.addInt32(FieldVectorOffset / IGM.getPointerSize());

‎lib/IRGen/GenReflection.cpp

+4-26
Original file line numberDiff line numberDiff line change
@@ -946,35 +946,13 @@ class FieldTypeMetadataBuilder : public ReflectionMetadataBuilder {
946946
B.addInt16(uint16_t(kind));
947947
B.addInt16(FieldRecordSize);
948948

949-
// Filter to select which fields we'll export FieldDescriptors for.
950-
auto exportable_field =
951-
[](Field field) {
952-
// Don't export private C++ fields that were imported as private Swift fields.
953-
// The type of a private field might not have all the type witness
954-
// operations that Swift requires, for instance,
955-
// `std::unique_ptr<IncompleteType>` would not have a destructor.
956-
if (field.getKind() == Field::Kind::Var &&
957-
field.getVarDecl()->getClangDecl() &&
958-
field.getVarDecl()->getFormalAccess() == AccessLevel::Private)
959-
return false;
960-
// All other fields are exportable
961-
return true;
962-
};
963-
964-
// Count exportable fields
965-
int exportableFieldCount = 0;
966-
forEachField(IGM, NTD, [&](Field field) {
967-
if (exportable_field(field)) {
968-
++exportableFieldCount;
969-
}
970-
});
971-
972949
// Emit exportable fields, prefixed with a count
973-
B.addInt32(exportableFieldCount);
950+
B.addInt32(countExportableFields(IGM, NTD));
951+
952+
// Filter to select which fields we'll export FieldDescriptor for.
974953
forEachField(IGM, NTD, [&](Field field) {
975-
if (exportable_field(field)) {
954+
if (isExportableField(field))
976955
addField(field);
977-
}
978956
});
979957
}
980958

‎lib/IRGen/StructLayout.cpp

+23
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,18 @@ unsigned irgen::getNumFields(const NominalTypeDecl *target) {
589589
return numFields;
590590
}
591591

592+
bool irgen::isPrivateField(VarDecl *decl) {
593+
return (decl->getClangDecl() &&
594+
decl->getFormalAccess() == AccessLevel::Private);
595+
}
596+
597+
bool irgen::isExportableField(Field field) {
598+
if (field.getKind() == Field::Kind::Var && isPrivateField(field.getVarDecl()))
599+
return false;
600+
// All other fields are exportable
601+
return true;
602+
}
603+
592604
void irgen::forEachField(IRGenModule &IGM, const NominalTypeDecl *typeDecl,
593605
llvm::function_ref<void(Field field)> fn) {
594606
auto classDecl = dyn_cast<ClassDecl>(typeDecl);
@@ -610,6 +622,17 @@ void irgen::forEachField(IRGenModule &IGM, const NominalTypeDecl *typeDecl,
610622
}
611623
}
612624

625+
unsigned irgen::countExportableFields(IRGenModule &IGM,
626+
const NominalTypeDecl *type) {
627+
// Don't count private C++ fields that were imported as private Swift fields
628+
int exportableFieldCount = 0;
629+
forEachField(IGM, type, [&](Field field) {
630+
if (isExportableField(field))
631+
++exportableFieldCount;
632+
});
633+
return exportableFieldCount;
634+
}
635+
613636
SILType Field::getType(IRGenModule &IGM, SILType baseType) const {
614637
switch (getKind()) {
615638
case Field::Var:

‎lib/IRGen/StructMetadataVisitor.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#ifndef SWIFT_IRGEN_STRUCTMETADATALAYOUT_H
1818
#define SWIFT_IRGEN_STRUCTMETADATALAYOUT_H
1919

20+
#include "Field.h"
2021
#include "NominalMetadataVisitor.h"
2122
#include "swift/AST/IRGenOptions.h"
2223

@@ -64,7 +65,8 @@ template <class Impl> class StructMetadataVisitor
6465
// Struct field offsets.
6566
asImpl().noteStartOfFieldOffsets();
6667
for (VarDecl *prop : Target->getStoredProperties())
67-
asImpl().addFieldOffset(prop);
68+
if (!isPrivateField(prop))
69+
asImpl().addFieldOffset(prop);
6870

6971
asImpl().noteEndOfFieldOffsets();
7072

‎test/Interop/Cxx/class/Inputs/module.modulemap

+6
Original file line numberDiff line numberDiff line change
@@ -152,3 +152,9 @@ module PIMPL {
152152
requires cplusplus
153153
export *
154154
}
155+
156+
module SimpleStructs {
157+
header "simple-structs.h"
158+
requires cplusplus
159+
export *
160+
}

‎test/Interop/Cxx/class/Inputs/simple-structs.h

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
#ifndef TEST_INTEROP_CXX_CLASS_INPUTS_SIMPLE_STRUCTS_H
2+
#define TEST_INTEROP_CXX_CLASS_INPUTS_SIMPLE_STRUCTS_H
3+
4+
#include <string>
5+
6+
struct HasPrivateFieldsOnly {
7+
private:
8+
int privInt;
9+
std::string privStr;
10+
11+
public:
12+
HasPrivateFieldsOnly(int i, std::string s) : privInt(i), privStr(s) {}
13+
};
14+
15+
struct HasPublicFieldsOnly {
16+
int pubInt;
17+
std::string pubStr;
18+
19+
HasPublicFieldsOnly(int i, std::string s) : pubInt(i), pubStr(s) {}
20+
};
21+
22+
struct HasIntFieldsOnly {
23+
private:
24+
int a = 1;
25+
26+
public:
27+
int b = 2;
28+
29+
private:
30+
int c = 3;
31+
32+
public:
33+
int d = 4;
34+
};
35+
36+
struct HasPrivateAndPublicFields {
37+
private:
38+
std::string privStr;
39+
40+
public:
41+
int pubInt;
42+
43+
private:
44+
int privInt;
45+
46+
public:
47+
std::string pubStr;
48+
49+
HasPrivateAndPublicFields(int i1, int i2, std::string s1, std::string s2)
50+
: privStr(s2), pubInt(i2), privInt(i1), pubStr(s1) {}
51+
};
52+
53+
#endif

‎test/Interop/Cxx/class/print.swift

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift %s -o %t/a.out -Xfrontend -enable-experimental-cxx-interop -I %S/Inputs
3+
// RUN: %target-codesign %t/a.out
4+
// RUN: %target-run %t/a.out | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-os
5+
6+
// REQUIRES: executable_test
7+
8+
import SimpleStructs
9+
10+
func printCxxStructPrivateFields() {
11+
let s = HasPrivateFieldsOnly(42, "Hello")
12+
print(s)
13+
}
14+
15+
func printCxxStructPublicFields() {
16+
let s = HasPublicFieldsOnly(42, "Hello")
17+
print(s)
18+
}
19+
20+
func printCxxStructIntFields() {
21+
let s = HasIntFieldsOnly()
22+
print(s)
23+
}
24+
25+
func printCxxStructPrivateAndPublicFields() {
26+
let s = HasPrivateAndPublicFields(24, 42, "Hello", "World")
27+
print(s)
28+
}
29+
30+
printCxxStructPrivateFields()
31+
// CHECK: HasPrivateFieldsOnly()
32+
33+
printCxxStructIntFields()
34+
// CHECK: HasIntFieldsOnly(b: 2, d: 4)
35+
36+
printCxxStructPublicFields()
37+
// CHECK-macosx: HasPublicFieldsOnly(pubInt: 42, pubStr: __C.std.std::__1.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>())
38+
// CHECK-linux-gnu: HasPublicFieldsOnly(pubInt: 42, pubStr: __C.std.std::__cxx11.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>())
39+
// CHECK-windows-msvc: HasPublicFieldsOnly(pubInt: 42, pubStr: __C.std.std::__1.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>())
40+
41+
printCxxStructPrivateAndPublicFields()
42+
// CHECK-macosx: HasPrivateAndPublicFields(pubInt: 42, pubStr: __C.std.std::__1.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>())
43+
// CHECK-linux-gnu: HasPrivateAndPublicFields(pubInt: 42, pubStr: __C.std.std::__cxx11.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>())
44+
// CHECK-windows-msvc: HasPrivateAndPublicFields(pubInt: 42, pubStr: __C.std.std::__1.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>())

‎test/Interop/Cxx/stdlib/Inputs/module.modulemap

+6
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ module MsvcUseVecIt {
6464
export *
6565
}
6666

67+
module StdlibTypes {
68+
header "stdlib-types.h"
69+
requires cplusplus
70+
export *
71+
}
72+
6773
module StdUniquePtr {
6874
header "std-unique-ptr.h"
6975
requires cplusplus

‎test/Interop/Cxx/stdlib/Inputs/stdlib-types.h

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
#ifndef TEST_INTEROP_CXX_STDLIB_INPUTS_STD_TYPES_H
2+
#define TEST_INTEROP_CXX_STDLIB_INPUTS_STD_TYPES_H
3+
4+
#include <memory>
5+
#include <string>
6+
#include <vector>
7+
8+
std::unique_ptr<int> makeInt() { return std::make_unique<int>(42); }
9+
10+
std::unique_ptr<std::string> makeString() {
11+
return std::make_unique<std::string>("Unique string");
12+
}
13+
14+
std::shared_ptr<int> makeIntShared() { return std::make_unique<int>(42); }
15+
16+
std::shared_ptr<std::string> makeStringShared() {
17+
return std::make_unique<std::string>("Shared string");
18+
}
19+
20+
std::vector<int> makeVecOfInt() { return {1, 2, 3}; }
21+
22+
std::vector<std::string> makeVecOfString() { return {"Hello", "World"}; }
23+
24+
#endif

‎test/Interop/Cxx/stdlib/print-stdlib-types.swift

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift %s -o %t/a.out -Xfrontend -enable-experimental-cxx-interop -I %S/Inputs
3+
// RUN: %target-codesign %t/a.out
4+
// RUN: %target-run %t/a.out | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-os
5+
6+
// REQUIRES: executable_test
7+
8+
import StdlibTypes
9+
10+
func printStdString(s: std.string) {
11+
print(s)
12+
let swiftString = String(s)
13+
print(swiftString)
14+
}
15+
16+
#if !os(Windows)
17+
// FIXME: We can't import std::unique_ptr or std::shared_ptr properly on Windows (https://github.com/apple/swift/issues/70226)
18+
func printStdUniquePtrOfInt() {
19+
let uint = makeInt()
20+
print(uint.pointee)
21+
}
22+
23+
func printStdUniquePtrOfString() {
24+
let ustring = makeString()
25+
print(ustring.pointee)
26+
}
27+
28+
func printStdSharedPtrOfInt() {
29+
let sint = makeIntShared()
30+
print(sint.pointee)
31+
print(sint)
32+
}
33+
34+
func printStdSharedPtrOfString() {
35+
let sstring = makeStringShared()
36+
print(sstring.pointee)
37+
print(sstring)
38+
}
39+
#endif
40+
41+
func printStdVectorOfInt() {
42+
let vecOfInt = makeVecOfInt()
43+
print(vecOfInt[0])
44+
print(vecOfInt)
45+
}
46+
47+
func printStdVectorOfString() {
48+
let vecOfString = makeVecOfString()
49+
print(vecOfString[0])
50+
print(vecOfString)
51+
}
52+
53+
printStdString(s: "Hello")
54+
// CHECK-macosx: basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>()
55+
// CHECK-linux-gnu: basic_string<CChar, std.char_traits<CChar>, std.allocator<CChar>>()
56+
// CHECK-windows-msvc: basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>()
57+
// CHECK: Hello
58+
59+
#if !os(Windows)
60+
printStdUniquePtrOfInt()
61+
// CHECK-macosx: 42
62+
// CHECK-linux-gnu: 42
63+
printStdUniquePtrOfString()
64+
// CHECK-macosx: basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>()
65+
// CHECK-linux-gnu: basic_string<CChar, std.char_traits<CChar>, std.allocator<CChar>>()
66+
printStdSharedPtrOfInt()
67+
// CHECK-macosx: 42
68+
// CHECK-macosx: shared_ptr<CInt>()
69+
// CHECK-linux-gnu: 42
70+
// CHECK-linux-gnu: shared_ptr<CInt>()
71+
printStdSharedPtrOfString()
72+
// CHECK-macosx: basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>()
73+
// CHECK-macosx: shared_ptr<std.__1.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>>()
74+
// CHECK-linux-gnu: basic_string<CChar, std.char_traits<CChar>, std.allocator<CChar>>()
75+
// CHECK-linux-gnu: shared_ptr<std.__cxx11.basic_string<CChar, std.char_traits<CChar>, std.allocator<CChar>>>()
76+
77+
#endif
78+
79+
printStdVectorOfInt()
80+
// CHECK: 1
81+
// CHECK-macosx: vector<CInt, std.__1.allocator<CInt>>()
82+
// CHECK-linux-gnu: vector<CInt, std.allocator<CInt>>()
83+
// CHECK-windows-msvc: vector<CInt, std.__1.allocator<CInt>>()
84+
printStdVectorOfString()
85+
// CHECK-macosx: basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>()
86+
// CHECK-macosx: vector<std.__1.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>, std.__1.allocator<std.__1.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>>>()
87+
// CHECK-linux-gnu: basic_string<CChar, std.char_traits<CChar>, std.allocator<CChar>>()
88+
// CHECK-linux-gnu: vector<std.__cxx11.basic_string<CChar, std.char_traits<CChar>, std.allocator<CChar>>, std.allocator<std.__cxx11.basic_string<CChar, std.char_traits<CChar>, std.allocator<CChar>>>>()
89+
// CHECK-windows-msvc: basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>()
90+
// CHECK-windows-msvc: vector<std.__1.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>, std.__1.allocator<std.__1.basic_string<CChar, std.__1.char_traits<CChar>, std.__1.allocator<CChar>>>>()
91+

0 commit comments

Comments
 (0)