Skip to content

Commit 02bc66b

Browse files
plotfiplotfi
committed
[cxx-interop] ClangDeclFinder to handle problematic codegen cases
At the moment, header defined functions do necessarily get codegened by C++-Interop if they are invoked purely on the call path from a constructor. What this means is that when you have inline or template functions that are header defined and are only invoked on a call graph path that only originates from a C++ constructor that is invoked from Swift, that you get LLVM IR Function declares instead of Function defines with linkonceodr. This results in link errors unless those functions happen to be invoked or template instantiated from another TU that gets linked in. This patch teaches the ClangDeclFinder to look for these cases and add them to the list of decls that get handled by HandleTopLevelDecl.
1 parent d48306c commit 02bc66b

File tree

4 files changed

+171
-0
lines changed

4 files changed

+171
-0
lines changed

‎lib/IRGen/GenClangDecl.cpp‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
#define DEBUG_TYPE "clang-decl-finder"
14+
1315
#include "IRGenModule.h"
1416
#include "swift/AST/ASTContext.h"
1517
#include "swift/AST/IRGenOptions.h"
@@ -82,6 +84,116 @@ clang::Decl *getDeclWithExecutableCode(clang::Decl *decl) {
8284
return nullptr;
8385
}
8486

87+
auto walkCallGraphFromCtor(
88+
const clang::CXXConstructorDecl *toplevelCtor,
89+
llvm::SmallPtrSet<const clang::Decl *, 8> visited,
90+
llvm::SmallPtrSet<const clang::Decl *, 8> &functionDecls) {
91+
92+
llvm::SmallVector<clang::Stmt *, 8> stack;
93+
auto recurseStmt = [&stack](clang::Stmt *s) {
94+
if (s) {
95+
stack.push_back(s);
96+
return true;
97+
}
98+
return false;
99+
};
100+
101+
// Keep track of decls we have already walked over so that we don't re-walk
102+
// over them redundantly. Returns true if the insertion took place.
103+
auto visitDecl = [&visited](const clang::Decl *decl) {
104+
return decl ? visited.insert(decl).second : false;
105+
};
106+
107+
auto handleCtor = [&recurseStmt,
108+
&functionDecls](const clang::CXXConstructorDecl *ctor) {
109+
functionDecls.insert(ctor);
110+
if (ctor->getParent()->getDestructor())
111+
functionDecls.insert(ctor->getParent()->getDestructor());
112+
recurseStmt(ctor->getBody());
113+
for (auto *init : ctor->inits()) {
114+
recurseStmt(init->getInit());
115+
if (clang::FieldDecl *field = init->getMember())
116+
recurseStmt(field->getInClassInitializer());
117+
}
118+
};
119+
120+
auto handleFunctionDecl = [&recurseStmt, &functionDecls](
121+
clang::FunctionDecl *functionDecl) {
122+
LLVM_DEBUG({
123+
llvm::dbgs() << "HANDLE FUNCTION DECL:\n";
124+
llvm::dbgs() << "IS INLINE "
125+
<< (functionDecl->isInlineSpecified() ? "YES" : "NO")
126+
<< "\n";
127+
llvm::dbgs() << "IS TEMPLATE INST "
128+
<< (functionDecl->isTemplateInstantiation() ? "YES" : "NO")
129+
<< "\n";
130+
});
131+
132+
if (functionDecl->isInlineSpecified() || // is 'inline' specified
133+
functionDecl->isInlined() || // is inlined or constexpr
134+
functionDecl->isTemplateInstantiation()) // is template instance
135+
functionDecls.insert(functionDecl);
136+
137+
// Even if this function is not inlined or a template instance we want to
138+
// recurse on it in case it calls something or calls something that calls
139+
// something that is.
140+
recurseStmt(functionDecl->getBody());
141+
};
142+
143+
handleCtor(toplevelCtor);
144+
145+
while (!stack.empty()) {
146+
auto *back = stack.pop_back_val();
147+
148+
LLVM_DEBUG({
149+
llvm::dbgs() << "\nClang Decl Walker Candidate:\n";
150+
back->dump(llvm::dbgs(), toplevelCtor->getASTContext());
151+
});
152+
153+
// Handle if the expression is a callsite or a ExprWithCleanups.
154+
if (auto *fn = dyn_cast<clang::MemberExpr>(back)) {
155+
if (visitDecl(fn->getMemberDecl()))
156+
recurseStmt(fn->getMemberDecl()->getBody());
157+
} else if (auto *fn = dyn_cast<clang::CallExpr>(back)) {
158+
auto *memberCall = dyn_cast<clang::CXXMemberCallExpr>(back);
159+
if (memberCall && visitDecl(memberCall->getMethodDecl())) {
160+
functionDecls.insert(memberCall->getMethodDecl());
161+
recurseStmt(memberCall->getMethodDecl()->getBody());
162+
}
163+
164+
if (visitDecl(fn->getCalleeDecl())) {
165+
functionDecls.insert(fn->getCalleeDecl());
166+
recurseStmt(fn->getCalleeDecl()->getBody());
167+
}
168+
} else if (auto *fn = dyn_cast<clang::CXXConstructExpr>(back)) {
169+
170+
// Use the constructor expression to traverse corresponding destructors.
171+
// This is done because default destructor decls are empty and have very
172+
// little to traverse.
173+
if (auto *dtor = fn->getConstructor() && fn->getConstructor()->getParent()
174+
? fn->getConstructor()->getParent()->getDestructor()
175+
: nullptr) {
176+
handleFunctionDecl(dtor);
177+
if (dtor->getBody())
178+
recurseStmt(dtor->getBody());
179+
}
180+
181+
for (auto *child : fn->children())
182+
recurseStmt(child);
183+
184+
if (visitDecl(fn->getConstructor()))
185+
handleCtor(fn->getConstructor());
186+
} else if (auto *declRef = dyn_cast<clang::DeclRefExpr>(back)) {
187+
if (auto *fn = dyn_cast_or_null<clang::FunctionDecl>(declRef->getDecl()))
188+
handleFunctionDecl(fn);
189+
}
190+
191+
// Walk the expression's children.
192+
for (clang::Stmt *child : back->children())
193+
recurseStmt(child);
194+
}
195+
}
196+
85197
} // end anonymous namespace
86198

87199
void IRGenModule::emitClangDecl(const clang::Decl *decl) {
@@ -120,6 +232,26 @@ void IRGenModule::emitClangDecl(const clang::Decl *decl) {
120232
stack.push_back(D);
121233
});
122234

235+
std::vector<const clang::Decl *> ctors;
236+
llvm::copy_if(stack, std::back_inserter(ctors), [](const clang::Decl *decl) {
237+
return isa<clang::CXXConstructorDecl>(decl);
238+
});
239+
240+
llvm::SmallPtrSet<const clang::Decl *, 8> visited;
241+
llvm::SmallPtrSet<const clang::Decl *, 8> functionDecls;
242+
for (auto *ctor : ctors)
243+
walkCallGraphFromCtor(cast<clang::CXXConstructorDecl>(ctor), visited,
244+
functionDecls);
245+
llvm::copy(functionDecls, std::back_inserter(stack));
246+
247+
LLVM_DEBUG({
248+
llvm::dbgs()
249+
<< "\nAdditional Function/Method decls found on constructor path:\n";
250+
for (auto *functionDecl : functionDecls)
251+
functionDecl->dump(llvm::dbgs());
252+
llvm::dbgs() << "\n";
253+
});
254+
123255
while (!stack.empty()) {
124256
auto *next = const_cast<clang::Decl *>(stack.pop_back_val());
125257
if (clang::Decl *executableDecl = getDeclWithExecutableCode(next)) {

‎test/Interop/Cxx/class/Inputs/initializer-from-inline.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
inline int get42() { return 42; }
2+
struct Hold42 { int m = get42(); };
3+
4+
template <typename T> T passThroughArgT(T t) { return t; }
5+
struct Hold23 { int m = passThroughArgT<int>(23); };
6+
7+
struct HoldMemberThatHolds42 { Hold42 m; };
8+
struct HoldMemberThatHoldsMemberThatHolds42 { HoldMemberThatHolds42 m; };
9+
10+
inline int get42Level4() { return get42(); }
11+
inline int get42Level3() { return get42Level4(); }
12+
inline int get42Level2() { return get42Level3(); }
13+
inline int get42Level1() { return get42Level2(); }
14+
struct Hold42WithLongInitCallGraph { int m = get42Level1(); };

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,8 @@ module InvalidNestedStruct {
8787
header "invalid-nested-struct.h"
8888
requires cplusplus
8989
}
90+
91+
module InitializerFromInline {
92+
header "initializer-from-inline.h"
93+
requires cplusplus
94+
}

‎test/Interop/Cxx/class/initializer-from-inline.swift‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: %swift -I %S/Inputs -enable-cxx-interop -emit-ir %s | %FileCheck %s
2+
3+
import InitializerFromInline
4+
5+
// CHECK: define linkonce_odr i32 @{{_Z5get42v|\?get42@@YAHXZ}}
6+
// CHECK: define linkonce_odr i32 @{{_Z15passThroughArgTIiET_S0_|\?\?$passThroughArgT@H@@YAHH@Z}}
7+
// CHECK: define linkonce_odr i32 @{{_Z11get42Level1v|\?get42Level1@@YAHXZ}}
8+
// CHECK: define linkonce_odr i32 @{{_Z11get42Level2v|\?get42Level2@@YAHXZ}}
9+
// CHECK: define linkonce_odr i32 @{{_Z11get42Level3v|\?get42Level3@@YAHXZ}}
10+
// CHECK: define linkonce_odr i32 @{{_Z11get42Level4v|\?get42Level4@@YAHXZ}}
11+
12+
var a = Hold42()
13+
var b = Hold23()
14+
var c = HoldMemberThatHolds42()
15+
var d = HoldMemberThatHoldsMemberThatHolds42()
16+
var e = Hold42WithLongInitCallGraph()
17+
18+
let sum = a.m + b.m + c.m.m + d.m.m.m + e.m
19+
20+
print("Sum: \(sum)")

0 commit comments

Comments
 (0)