Skip to content

Commit c28992a

Browse files
authored
[Clang] Allow simpler visibility annotations when targeting win32 and mingw (#133699)
MinGW and Win32 disagree on where the `__declspec(dllexport)` should be placed on extern template instantiations. However, there doesn't fundamentally seem to be a problem with putting the annotation in both places. This patch adds a new diagnostic group and `-Wattribute-ignored` warnings about where the attribute is placed if the attribute is different on the declaration and definition. There is another new warning group `-Wdllexport-explicit-instantiation` that also diagnoses places where the attribute is technically ignored, even though the correct place is also annotated. This makes it possible to significantly simplify libc++'s visibility annotations (see #133704).
1 parent 60b5d06 commit c28992a

File tree

7 files changed

+66
-6
lines changed

7 files changed

+66
-6
lines changed

clang/include/clang/Basic/Attr.td

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4397,6 +4397,14 @@ def DLLExport : InheritableAttr, TargetSpecificAttr<TargetHasDLLImportExport> {
43974397
let Documentation = [DLLExportDocs];
43984398
}
43994399

4400+
def DLLExportOnDecl : InheritableAttr, TargetSpecificAttr<TargetHasDLLImportExport> {
4401+
// This attribute is only used to warn if there was a `__declspec(dllexport)`
4402+
// on a declaration, but not on the defintion of an explicit instantiation
4403+
let Spellings = [];
4404+
let Subjects = SubjectList<[CXXRecord]>;
4405+
let Documentation = [InternalOnly];
4406+
}
4407+
44004408
def DLLExportStaticLocal : InheritableAttr, TargetSpecificAttr<TargetHasDLLImportExport> {
44014409
// This attribute is used internally only when -fno-dllexport-inlines is
44024410
// passed. This attribute is added to inline functions of a class having the

clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,9 @@ def LifetimeSafetySuggestions
546546

547547
def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
548548
def DllexportExplicitInstantiationDecl : DiagGroup<"dllexport-explicit-instantiation-decl">;
549+
def DllexportExplicitInstantiation :
550+
DiagGroup<"dllexport-explicit-instantiation",
551+
[DllexportExplicitInstantiationDecl]>;
549552
def ExcessInitializers : DiagGroup<"excess-initializers">;
550553
def ExpansionToDefined : DiagGroup<"expansion-to-defined">;
551554
def FlagEnum : DiagGroup<"flag-enum">;
@@ -990,7 +993,8 @@ def NSReturnsMismatch : DiagGroup<"nsreturns-mismatch">;
990993

991994
def IndependentClassAttribute : DiagGroup<"IndependentClass-attribute">;
992995
def UnknownAttributes : DiagGroup<"unknown-attributes">;
993-
def IgnoredAttributes : DiagGroup<"ignored-attributes">;
996+
def IgnoredAttributes : DiagGroup<"ignored-attributes",
997+
[DllexportExplicitInstantiation]>;
994998
def Attributes : DiagGroup<"attributes", [UnknownAttributes,
995999
IgnoredAttributes]>;
9961000
def UnknownSanitizers : DiagGroup<"unknown-sanitizers">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3904,9 +3904,19 @@ def warn_attribute_dllimport_static_field_definition : Warning<
39043904
def warn_attribute_dllexport_explicit_instantiation_decl : Warning<
39053905
"explicit instantiation declaration should not be 'dllexport'">,
39063906
InGroup<DllexportExplicitInstantiationDecl>;
3907-
def warn_attribute_dllexport_explicit_instantiation_def : Warning<
3907+
def warn_attr_dllexport_explicit_inst_def : Warning<
39083908
"'dllexport' attribute ignored on explicit instantiation definition">,
3909+
InGroup<DllexportExplicitInstantiation>;
3910+
def warn_attr_dllexport_explicit_inst_def_mismatch : Warning<
3911+
"'dllexport' attribute ignored on explicit instantiation definition">,
3912+
InGroup<IgnoredAttributes>;
3913+
def note_prev_decl_missing_dllexport : Note<
3914+
"'dllexport' attribute is missing on previous declaration">;
3915+
def warn_dllexport_on_decl_ignored : Warning<
3916+
"explicit instantiation definition is not exported without 'dllexport'">,
39093917
InGroup<IgnoredAttributes>;
3918+
def note_dllexport_on_decl : Note<
3919+
"'dllexport' attribute on the declaration is ignored">;
39103920
def warn_attribute_exclude_from_explicit_instantiation_local_class : Warning<
39113921
"%0 attribute ignored on local class%select{| member}1">,
39123922
InGroup<IgnoredAttributes>;

clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6590,7 +6590,10 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
65906590
if (ClassExported && !ClassAttr->isInherited() &&
65916591
TSK == TSK_ExplicitInstantiationDeclaration &&
65926592
!Context.getTargetInfo().getTriple().isOSCygMing()) {
6593-
Class->dropAttr<DLLExportAttr>();
6593+
if (auto *DEA = Class->getAttr<DLLExportAttr>()) {
6594+
Class->addAttr(DLLExportOnDeclAttr::Create(Context, DEA->getLoc()));
6595+
Class->dropAttr<DLLExportAttr>();
6596+
}
65946597
return;
65956598
}
65966599

clang/lib/Sema/SemaTemplate.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10282,13 +10282,29 @@ DeclResult Sema::ActOnExplicitInstantiation(
1028210282
// mode, if a previous declaration of the instantiation was seen.
1028310283
for (const ParsedAttr &AL : Attr) {
1028410284
if (AL.getKind() == ParsedAttr::AT_DLLExport) {
10285-
Diag(AL.getLoc(),
10286-
diag::warn_attribute_dllexport_explicit_instantiation_def);
10285+
if (PrevDecl->hasAttr<DLLExportAttr>()) {
10286+
Diag(AL.getLoc(), diag::warn_attr_dllexport_explicit_inst_def);
10287+
} else {
10288+
Diag(AL.getLoc(),
10289+
diag::warn_attr_dllexport_explicit_inst_def_mismatch);
10290+
Diag(PrevDecl->getLocation(), diag::note_prev_decl_missing_dllexport);
10291+
}
1028710292
break;
1028810293
}
1028910294
}
1029010295
}
1029110296

10297+
if (TSK == TSK_ExplicitInstantiationDefinition && PrevDecl &&
10298+
!Context.getTargetInfo().getTriple().isWindowsGNUEnvironment() &&
10299+
llvm::none_of(Attr, [](const ParsedAttr &AL) {
10300+
return AL.getKind() == ParsedAttr::AT_DLLExport;
10301+
})) {
10302+
if (const auto *DEA = PrevDecl->getAttr<DLLExportOnDeclAttr>()) {
10303+
Diag(TemplateLoc, diag::warn_dllexport_on_decl_ignored);
10304+
Diag(DEA->getLoc(), diag::note_dllexport_on_decl);
10305+
}
10306+
}
10307+
1029210308
if (CheckExplicitInstantiation(*this, ClassTemplate, TemplateNameLoc,
1029310309
SS.isSet(), TSK))
1029410310
return true;
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify=win32,win32-pedantic %s -Wdllexport-explicit-instantiation
2+
// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify=win32 %s -Wno-dllexport-explicit-instantiation
3+
// RUN: %clang_cc1 -triple x86_64-mingw32 -fsyntax-only -fms-extensions -verify=mingw,mingw-pedantic %s -Wdllexport-explicit-instantiation
4+
// RUN: %clang_cc1 -triple x86_64-mingw32 -fsyntax-only -fms-extensions -verify=mingw %s -Wno-dllexport-explicit-instantiation
5+
6+
template <class>
7+
class S {};
8+
9+
extern template class __declspec(dllexport) S<short>; // win32-pedantic-warning {{explicit instantiation declaration should not be 'dllexport'}} \
10+
win32-pedantic-note {{attribute is here}}
11+
template class __declspec(dllexport) S<short>; // mingw-pedantic-warning {{'dllexport' attribute ignored on explicit instantiation definition}}
12+
13+
extern template class __declspec(dllexport) S<int>; // win32-pedantic-warning {{explicit instantiation declaration should not be 'dllexport'}} \
14+
win32-pedantic-note {{attribute is here}} \
15+
win32-note {{'dllexport' attribute on the declaration is ignored}}
16+
template class S<int>; // win32-warning {{explicit instantiation definition is not exported without 'dllexport'}}
17+
18+
extern template class S<long>; // mingw-note {{'dllexport' attribute is missing on previous declaration}}
19+
template class __declspec(dllexport) S<long>; // mingw-warning {{'dllexport' attribute ignored on explicit instantiation definition}}

clang/test/SemaCXX/dllexport.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ template struct ExplicitlyInstantiatedTemplate<int>; // ms-ps-note {{class templ
430430
template <typename T> struct ExplicitlyExportInstantiatedTemplate { void func() {} };
431431
template struct __declspec(dllexport) ExplicitlyExportInstantiatedTemplate<int>;
432432
template <typename T> struct ExplicitlyExportDeclaredInstantiatedTemplate { void func() {} };
433-
extern template struct ExplicitlyExportDeclaredInstantiatedTemplate<int>;
433+
extern template struct ExplicitlyExportDeclaredInstantiatedTemplate<int>; // gnu-note {{attribute is missing}}
434434
template struct __declspec(dllexport) ExplicitlyExportDeclaredInstantiatedTemplate<int>; // gnu-warning {{'dllexport' attribute ignored on explicit instantiation definition}}
435435
template <typename T> struct ExplicitlyImportInstantiatedTemplate { void func() {} };
436436
template struct __declspec(dllimport) ExplicitlyImportInstantiatedTemplate<int>;

0 commit comments

Comments
 (0)