Skip to content

Commit 9b0c17e

Browse files
zoecarverzoecarver
authored
Merge pull request swiftlang#34869 from zoecarver/cxx/fix/forward-declared-with-child
[cxx-interop] Use cached record when possible.
2 parents 10160cb + ecd1018 commit 9b0c17e

File tree

4 files changed

+137
-40
lines changed

4 files changed

+137
-40
lines changed

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 51 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3300,12 +3300,19 @@ namespace {
33003300

33013301
// Create the struct declaration and record it.
33023302
auto name = importedName.getDeclName().getBaseIdentifier();
3303-
auto result = Impl.createDeclWithClangNode<StructDecl>(decl,
3304-
AccessLevel::Public,
3305-
Impl.importSourceLoc(decl->getBeginLoc()),
3306-
name,
3307-
Impl.importSourceLoc(decl->getLocation()),
3308-
None, nullptr, dc);
3303+
StructDecl *result = nullptr;
3304+
// Try to find an already-imported struct. This case happens any time
3305+
// there are nested structs. The "Parent" struct will import the "Child"
3306+
// struct at which point it attempts to import its decl context which is
3307+
// the "Parent" struct. Without trying to look up already-imported structs
3308+
// this will cause an infinite loop.
3309+
auto alreadyImportedResult =
3310+
Impl.ImportedDecls.find({decl->getCanonicalDecl(), getVersion()});
3311+
if (alreadyImportedResult != Impl.ImportedDecls.end())
3312+
return alreadyImportedResult->second;
3313+
result = Impl.createDeclWithClangNode<StructDecl>(
3314+
decl, AccessLevel::Public, Impl.importSourceLoc(decl->getBeginLoc()),
3315+
name, Impl.importSourceLoc(decl->getLocation()), None, nullptr, dc);
33093316
Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result;
33103317

33113318
// FIXME: Figure out what to do with superclasses in C++. One possible
@@ -3316,6 +3323,7 @@ namespace {
33163323
SmallVector<VarDecl *, 4> members;
33173324
SmallVector<FuncDecl *, 4> methods;
33183325
SmallVector<ConstructorDecl *, 4> ctors;
3326+
SmallVector<TypeDecl *, 4> nestedTypes;
33193327

33203328
// FIXME: Import anonymous union fields and support field access when
33213329
// it is nested in a struct.
@@ -3374,27 +3382,13 @@ namespace {
33743382
continue;
33753383
}
33763384

3377-
if (isa<TypeDecl>(member)) {
3385+
if (auto nestedType = dyn_cast<TypeDecl>(member)) {
33783386
// Only import definitions. Otherwise, we might add the same member
33793387
// twice.
33803388
if (auto tagDecl = dyn_cast<clang::TagDecl>(nd))
33813389
if (tagDecl->getDefinition() != tagDecl)
33823390
continue;
3383-
// A struct nested inside another struct will either be logically
3384-
// a sibling of the outer struct, or contained inside of it, depending
3385-
// on if it has a declaration name or not.
3386-
//
3387-
// struct foo { struct bar { ... } baz; } // sibling
3388-
// struct foo { struct { ... } baz; } // child
3389-
//
3390-
// In the latter case, we add the imported type as a nested type
3391-
// of the parent.
3392-
//
3393-
// TODO: C++ types have different rules.
3394-
if (auto nominalDecl = dyn_cast<NominalTypeDecl>(member->getDeclContext())) {
3395-
assert(nominalDecl == result && "interesting nesting of C types?");
3396-
nominalDecl->addMember(member);
3397-
}
3391+
nestedTypes.push_back(nestedType);
33983392
continue;
33993393
}
34003394

@@ -3417,7 +3411,30 @@ namespace {
34173411
}
34183412

34193413
members.push_back(VD);
3414+
}
34203415

3416+
for (auto nestedType : nestedTypes) {
3417+
// A struct nested inside another struct will either be logically
3418+
// a sibling of the outer struct, or contained inside of it, depending
3419+
// on if it has a declaration name or not.
3420+
//
3421+
// struct foo { struct bar { ... } baz; } // sibling
3422+
// struct foo { struct { ... } baz; } // child
3423+
//
3424+
// In the latter case, we add the imported type as a nested type
3425+
// of the parent.
3426+
//
3427+
// TODO: C++ types have different rules.
3428+
if (auto nominalDecl =
3429+
dyn_cast<NominalTypeDecl>(nestedType->getDeclContext())) {
3430+
assert(nominalDecl == result && "interesting nesting of C types?");
3431+
nominalDecl->addMember(nestedType);
3432+
}
3433+
}
3434+
3435+
bool hasReferenceableFields = !members.empty();
3436+
for (auto member : members) {
3437+
auto nd = cast<clang::NamedDecl>(member->getClangDecl());
34213438
// Bitfields are imported as computed properties with Clang-generated
34223439
// accessors.
34233440
bool isBitField = false;
@@ -3428,37 +3445,33 @@ namespace {
34283445
hasUnreferenceableStorage = true;
34293446
isBitField = true;
34303447

3431-
makeBitFieldAccessors(Impl,
3432-
const_cast<clang::RecordDecl *>(decl),
3433-
result,
3434-
const_cast<clang::FieldDecl *>(field),
3435-
VD);
3448+
makeBitFieldAccessors(Impl, const_cast<clang::RecordDecl *>(decl),
3449+
result, const_cast<clang::FieldDecl *>(field),
3450+
member);
34363451
}
34373452
}
34383453

34393454
if (auto ind = dyn_cast<clang::IndirectFieldDecl>(nd)) {
34403455
// Indirect fields are created as computed property accessible the
34413456
// fields on the anonymous field from which they are injected.
3442-
makeIndirectFieldAccessors(Impl, ind, members, result, VD);
3457+
makeIndirectFieldAccessors(Impl, ind, members, result, member);
34433458
} else if (decl->isUnion() && !isBitField) {
34443459
// Union fields should only be available indirectly via a computed
34453460
// property. Since the union is made of all of the fields at once,
34463461
// this is a trivial accessor that casts self to the correct
34473462
// field type.
3448-
makeUnionFieldAccessors(Impl, result, VD);
3463+
makeUnionFieldAccessors(Impl, result, member);
34493464

34503465
// Create labeled initializers for unions that take one of the
34513466
// fields, which only initializes the data for that field.
3452-
auto valueCtor =
3453-
createValueConstructor(Impl, result, VD,
3454-
/*want param names*/true,
3455-
/*wantBody=*/true);
3467+
auto valueCtor = createValueConstructor(Impl, result, member,
3468+
/*want param names*/ true,
3469+
/*wantBody=*/true);
34563470
ctors.push_back(valueCtor);
34573471
}
3472+
result->addMember(member);
34583473
}
34593474

3460-
bool hasReferenceableFields = !members.empty();
3461-
34623475
const clang::CXXRecordDecl *cxxRecordDecl =
34633476
dyn_cast<clang::CXXRecordDecl>(decl);
34643477
if (hasZeroInitializableStorage && !cxxRecordDecl) {
@@ -3486,10 +3499,6 @@ namespace {
34863499
ctors.push_back(valueCtor);
34873500
}
34883501

3489-
for (auto member : members) {
3490-
result->addMember(member);
3491-
}
3492-
34933502
for (auto ctor : ctors) {
34943503
result->addMember(ctor);
34953504
}
@@ -9033,7 +9042,9 @@ ClangImporter::Implementation::loadAllMembers(Decl *D, uint64_t extra) {
90339042
if (!member)
90349043
continue;
90359044

9036-
enumDecl->addMember(member);
9045+
// TODO: remove this change when #34706 lands.
9046+
if (!member->NextDecl)
9047+
enumDecl->addMember(member);
90379048
}
90389049
}
90399050
return;

‎test/Interop/Cxx/class/Inputs/linked-records.h

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#ifndef TEST_INTEROP_CXX_CLASS_INPUTS_LINKED_RECORDS_H
2+
#define TEST_INTEROP_CXX_CLASS_INPUTS_LINKED_RECORDS_H
3+
4+
namespace Space {
5+
6+
class C;
7+
8+
struct A {
9+
struct B {
10+
B(int) {}
11+
B(char) {}
12+
B(const C *) {}
13+
};
14+
};
15+
16+
struct C {
17+
struct D {
18+
A::B B;
19+
};
20+
};
21+
22+
struct E {
23+
static void test(const C *);
24+
};
25+
26+
} // namespace Space
27+
28+
struct M {};
29+
30+
struct F {
31+
union {
32+
struct {
33+
} c;
34+
M m;
35+
};
36+
M m2;
37+
};
38+
39+
#endif // TEST_INTEROP_CXX_CLASS_INPUTS_LINKED_RECORDS_H

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,7 @@ module NestedRecords {
5757
header "nested-records.h"
5858
requires cplusplus
5959
}
60+
61+
module LinkedRecords {
62+
header "linked-records.h"
63+
}

‎test/Interop/Cxx/class/linked-records-module-interface.swift

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// RUN: %target-swift-ide-test -print-module -module-to-print=LinkedRecords -I %S/Inputs/ -source-filename=x -enable-cxx-interop | %FileCheck %s
2+
3+
// CHECK: enum Space {
4+
// CHECK: struct C {
5+
// CHECK: struct D {
6+
// CHECK: var B: Space.A.B
7+
// CHECK: init(B: Space.A.B)
8+
// CHECK: }
9+
// CHECK: }
10+
// CHECK: struct A {
11+
// CHECK: struct B {
12+
// CHECK: init(_: Int32)
13+
// CHECK: init(_: CChar)
14+
// CHECK: }
15+
// CHECK: init()
16+
// CHECK: }
17+
// CHECK: struct E {
18+
// CHECK: init()
19+
// CHECK: static func test(_: UnsafePointer<Space.C>!)
20+
// CHECK: }
21+
// CHECK: }
22+
23+
// CHECK: struct M {
24+
// CHECK: init()
25+
// CHECK: }
26+
// CHECK: struct F {
27+
// CHECK: struct __Unnamed_union___Anonymous_field0 {
28+
// CHECK: struct __Unnamed_struct_c {
29+
// CHECK: init()
30+
// CHECK: }
31+
// CHECK: var c: F.__Unnamed_union___Anonymous_field0.__Unnamed_struct_c
32+
// CHECK: var m: M
33+
// CHECK: init()
34+
// CHECK: init(c: F.__Unnamed_union___Anonymous_field0.__Unnamed_struct_c)
35+
// CHECK: init(m: M)
36+
// CHECK: }
37+
// CHECK: var __Anonymous_field0: F.__Unnamed_union___Anonymous_field0
38+
// CHECK: var c: F.__Unnamed_union___Anonymous_field0.__Unnamed_struct_c
39+
// CHECK: var m: M
40+
// CHECK: var m2: M
41+
// CHECK: init()
42+
// CHECK: init(_ __Anonymous_field0: F.__Unnamed_union___Anonymous_field0, m2: M)
43+
// CHECK: }

0 commit comments

Comments
 (0)