Skip to content

Commit 8c9c685

Browse files
andrurogerzcompnerd
andrurogerz
and
compnerd
authored
only export specialized template class declarations (#47)
## Purpose Fix class-level export annotation handling for template class declarations. ## Overview This patch makes the following behavioral changes to IDS: 1. Do not add class-level annotations to template class declarations. 2. Ensure class-level annotations are added to fully specialized template class declarations. ## Validation 1. New test cases to verify only fully-specialized template class declarations with virtual methods are annotated for export. 2. Ran tests locally on Windows and Linux, --------- Co-authored-by: Saleem Abdulrasool <[email protected]>
1 parent b1803f0 commit 8c9c685

File tree

2 files changed

+153
-17
lines changed

2 files changed

+153
-17
lines changed

‎Sources/idt/idt.cc

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,19 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
241241
return false;
242242
}
243243

244-
public:
245-
visitor(clang::ASTContext &context, PPCallbacks::FileIncludes &file_includes)
246-
: context_(context), source_manager_(context.getSourceManager()),
247-
file_includes_(file_includes) {}
244+
// Determine if a tagged type needs exporting at the record level.
245+
// Returns true if the record has been, or was already, annotated. Returns
246+
// false otherwise.
247+
bool export_record_if_needed(clang::CXXRecordDecl *RD) {
248+
// Check if the class is already exported.
249+
if (is_symbol_exported(RD))
250+
return true;
251+
252+
// Skip exporting template classes. For fully-specialized template classes,
253+
// isTemplated() returns false so they will be annotated if needed.
254+
if (RD->isTemplated())
255+
return false;
248256

249-
bool TraverseCXXRecordDecl(clang::CXXRecordDecl *RD) {
250257
// If a class declaration contains an out-of-line virtual method, annotate
251258
// the class instead of its individual members. This ensures its vtable is
252259
// exported on non-Windows platforms. Do this regardless of the method's
@@ -258,22 +265,31 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
258265
(MD->isVirtual() && !MD->hasBody())))
259266
break;
260267

261-
const bool is_exported_record = is_symbol_exported(RD);
262-
if (!is_exported_record && should_export_record) {
263-
// Insert the annotation immediately before the tag name, which is the
264-
// position returned by getLocation.
265-
clang::LangOptions LO = RD->getASTContext().getLangOpts();
266-
clang::SourceLocation SLoc = RD->getLocation();
267-
const clang::SourceLocation location =
268-
context_.getFullLoc(SLoc).getExpansionLoc();
269-
unexported_public_interface(location)
270-
<< RD << clang::FixItHint::CreateInsertion(SLoc, export_macro + " ");
271-
}
268+
if (!should_export_record)
269+
return false;
270+
271+
// Insert the annotation immediately before the tag name, which is the
272+
// position returned by getLocation.
273+
clang::LangOptions LO = RD->getASTContext().getLangOpts();
274+
clang::SourceLocation SLoc = RD->getLocation();
275+
const clang::SourceLocation location =
276+
context_.getFullLoc(SLoc).getExpansionLoc();
277+
unexported_public_interface(location)
278+
<< RD << clang::FixItHint::CreateInsertion(SLoc, export_macro + " ");
272279

280+
return true;
281+
}
282+
283+
public:
284+
visitor(clang::ASTContext &context, PPCallbacks::FileIncludes &file_includes)
285+
: context_(context), source_manager_(context.getSourceManager()),
286+
file_includes_(file_includes) {}
287+
288+
bool TraverseCXXRecordDecl(clang::CXXRecordDecl *RD) {
273289
// Save/restore the current value of in_exported_record_ to support nested
274290
// record definitions.
275291
const bool old_in_exported_record = in_exported_record_;
276-
in_exported_record_ = is_exported_record || should_export_record;
292+
in_exported_record_ = export_record_if_needed(RD);
277293

278294
// Traverse the class by invoking the parent's version of this method. This
279295
// call is required even if the record is exported because it may contain
@@ -283,6 +299,30 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
283299
return result;
284300
}
285301

302+
// RecursiveASTVisitor::TraverseCXXRecordDecl does not get called for fully
303+
// specialized template declarations. Since we may want to export them,
304+
// manually invoke TraverseCXXRecordDecl whenever an explicit specialization
305+
// is found.
306+
bool TraverseClassTemplateSpecializationDecl(
307+
clang::ClassTemplateSpecializationDecl *SD) {
308+
switch (SD->getSpecializationKind()) {
309+
case clang::TSK_ExplicitSpecialization:
310+
// This call visits class template specialization record and recursively
311+
// visits all of it children, which may also require export.
312+
return TraverseCXXRecordDecl(SD);
313+
314+
// TODO: consider annotating explicit template instantiation declarations
315+
// and definitions in the future. They may require unique annotation macros
316+
// due to differences between visibility and dllexport/dllimport attributes.
317+
case clang::TSK_ExplicitInstantiationDeclaration:
318+
[[fallthrough]];
319+
case clang::TSK_ExplicitInstantiationDefinition:
320+
[[fallthrough]];
321+
default:
322+
return true;
323+
}
324+
}
325+
286326
bool VisitFunctionDecl(clang::FunctionDecl *FD) {
287327
clang::FullSourceLoc location = get_location(FD);
288328

‎Tests/TemplateClasses.hh

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// RUN: %idt --export-macro=IDT_TEST_ABI %s 2>&1 | %FileCheck %s
2+
3+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
4+
template <typename T, typename U> struct TemplateClassVirtual {
5+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
6+
virtual void virtualMethod();
7+
8+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
9+
static long staticField;
10+
};
11+
12+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
13+
template <typename U> struct TemplateClassVirtual<int, U> {
14+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
15+
virtual void virtualMethod();
16+
17+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
18+
static long staticField;
19+
};
20+
21+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
22+
extern template class TemplateClassVirtual<long, void*>;
23+
24+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
25+
template class TemplateClassVirtual<int, long>;
26+
27+
// CHECK: TemplateClasses.hh:[[@LINE+1]]:20: remark: unexported public interface 'TemplateClassVirtual<long, int>'
28+
template <> struct TemplateClassVirtual<long, int> {
29+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
30+
virtual void virtualMethod();
31+
32+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
33+
static long staticField;
34+
};
35+
36+
// CHECK: TemplateClasses.hh:[[@LINE+1]]:20: remark: unexported public interface 'TemplateClassVirtual<char, int>'
37+
template <> struct TemplateClassVirtual<char, int> {
38+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
39+
virtual void virtualMethod();
40+
41+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
42+
static long staticField;
43+
44+
// CHECK: TemplateClasses.hh:[[@LINE+1]]:10: remark: unexported public interface 'NestedClass'
45+
struct NestedClass {
46+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
47+
virtual void virtualMethod();
48+
49+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
50+
static long staticField;
51+
};
52+
};
53+
54+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
55+
template <typename T, typename U> struct TemplateClass {
56+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
57+
void method();
58+
59+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
60+
static long staticField;
61+
};
62+
63+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
64+
template <typename U> struct TemplateClass<int, U> {
65+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
66+
void method();
67+
68+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
69+
static long staticField;
70+
};
71+
72+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
73+
template <> struct TemplateClass<int, long> {
74+
// CHECK: TemplateClasses.hh:[[@LINE+1]]:3: remark: unexported public interface 'method'
75+
void method();
76+
77+
// CHECK: TemplateClasses.hh:[[@LINE+1]]:{{.*}}: remark: unexported public interface 'staticField'
78+
static long staticField;
79+
};
80+
81+
// CHECK-NOT: TemplateClasses.hh:[[@LINE+1]]:{{.*}}
82+
template <> struct TemplateClass<char, int> {
83+
// CHECK: TemplateClasses.hh:[[@LINE+1]]:3: remark: unexported public interface 'method'
84+
void method();
85+
86+
// CHECK: TemplateClasses.hh:[[@LINE+1]]:{{.*}}: remark: unexported public interface 'staticField'
87+
static long staticField;
88+
89+
struct NestedClass {
90+
// CHECK: TemplateClasses.hh:[[@LINE+1]]:5: remark: unexported public interface 'method'
91+
void method();
92+
93+
// CHECK: TemplateClasses.hh:[[@LINE+1]]:{{.*}}: remark: unexported public interface 'staticField'
94+
static long staticField;
95+
};
96+
};

0 commit comments

Comments
 (0)