[clang] [Clang] Allow simpler visibility annotations when targeting win32 and mingw (PR #133699)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 11 06:31:52 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Nikolas Klauser (philnik777)
<details>
<summary>Changes</summary>
MinGW and Win32 disagree on where the `__declspec(dllexport)` should be placed. 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 attribue is technically ignored,
even though the correct place is also annotated. This makes it possible to significantly simplify libc++'s
visibility annotations (see #<!-- -->133704).
---
Full diff: https://github.com/llvm/llvm-project/pull/133699.diff
7 Files Affected:
- (modified) clang/include/clang/Basic/Attr.td (+8)
- (modified) clang/include/clang/Basic/DiagnosticGroups.td (+5-1)
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+11-1)
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+4-1)
- (modified) clang/lib/Sema/SemaTemplate.cpp (+18-2)
- (added) clang/test/SemaCXX/dllexport-explicit-instantiation.cpp (+19)
- (modified) clang/test/SemaCXX/dllexport.cpp (+3)
``````````diff
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 0999d8065e9f5..6bb8fc05b119a 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4185,6 +4185,14 @@ def DLLExport : InheritableAttr, TargetSpecificAttr<TargetHasDLLImportExport> {
let Documentation = [DLLExportDocs];
}
+def DLLExportOnDecl : InheritableAttr, TargetSpecificAttr<TargetHasDLLImportExport> {
+ // This attribute is only used to warn if there was a `__declspec(dllexport)`
+ // on a declaration, but not on the defintion of an explciit instantiation
+ let Spellings = [];
+ let Subjects = SubjectList<[CXXRecord]>;
+ let Documentation = [InternalOnly];
+}
+
def DLLExportStaticLocal : InheritableAttr, TargetSpecificAttr<TargetHasDLLImportExport> {
// This attribute is used internally only when -fno-dllexport-inlines is
// passed. This attribute is added to inline functions of a class having the
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index b9f08d96151c9..80f27771248bb 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -487,6 +487,9 @@ def Dangling : DiagGroup<"dangling", [DanglingAssignment,
ReturnStackAddress]>;
def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
def DllexportExplicitInstantiationDecl : DiagGroup<"dllexport-explicit-instantiation-decl">;
+def DllexportExplicitInstantiation :
+ DiagGroup<"dllexport-explicit-instantiation",
+ [DllexportExplicitInstantiationDecl]>;
def ExcessInitializers : DiagGroup<"excess-initializers">;
def ExpansionToDefined : DiagGroup<"expansion-to-defined">;
def FlagEnum : DiagGroup<"flag-enum">;
@@ -870,7 +873,8 @@ def NSReturnsMismatch : DiagGroup<"nsreturns-mismatch">;
def IndependentClassAttribute : DiagGroup<"IndependentClass-attribute">;
def UnknownAttributes : DiagGroup<"unknown-attributes">;
-def IgnoredAttributes : DiagGroup<"ignored-attributes">;
+def IgnoredAttributes : DiagGroup<"ignored-attributes",
+ [DllexportExplicitInstantiation]>;
def Attributes : DiagGroup<"attributes", [UnknownAttributes,
IgnoredAttributes]>;
def UnknownSanitizers : DiagGroup<"unknown-sanitizers">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1e900437d41ce..31324b6b1262f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3736,9 +3736,19 @@ def warn_attribute_dllimport_static_field_definition : Warning<
def warn_attribute_dllexport_explicit_instantiation_decl : Warning<
"explicit instantiation declaration should not be 'dllexport'">,
InGroup<DllexportExplicitInstantiationDecl>;
-def warn_attribute_dllexport_explicit_instantiation_def : Warning<
+def warn_attr_dllexport_explicit_inst_def : Warning<
"'dllexport' attribute ignored on explicit instantiation definition">,
+ InGroup<DllexportExplicitInstantiation>;
+def warn_attr_dllexport_explicit_inst_def_mismatch : Warning<
+ "'dllexport' attribute ignored on explicit instantiation definition">,
+ InGroup<IgnoredAttributes>;
+def note_prev_decl_missing_dllexport : Note<
+ "'dllexport' attribute is missing on previous declaration">;
+def warn_dllexport_on_decl_ignored : Warning<
+ "explicit instantiation definition is not exported without 'dllexport'">,
InGroup<IgnoredAttributes>;
+def note_dllexport_on_decl : Note<
+ "'dllexport' attribute on the declaration is ignored">;
def warn_attribute_exclude_from_explicit_instantiation_local_class : Warning<
"%0 attribute ignored on local class%select{| member}1">,
InGroup<IgnoredAttributes>;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 676d53a1f4b45..4d69c11678620 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -6606,7 +6606,10 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
if (ClassExported && !ClassAttr->isInherited() &&
TSK == TSK_ExplicitInstantiationDeclaration &&
!Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) {
- Class->dropAttr<DLLExportAttr>();
+ if (auto *DEA = Class->getAttr<DLLExportAttr>()) {
+ Class->addAttr(DLLExportOnDeclAttr::Create(Context, DEA->getLoc()));
+ Class->dropAttr<DLLExportAttr>();
+ }
return;
}
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index be81b6a46b2c0..25d84fde65970 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -9845,13 +9845,29 @@ DeclResult Sema::ActOnExplicitInstantiation(
// mode, if a previous declaration of the instantiation was seen.
for (const ParsedAttr &AL : Attr) {
if (AL.getKind() == ParsedAttr::AT_DLLExport) {
- Diag(AL.getLoc(),
- diag::warn_attribute_dllexport_explicit_instantiation_def);
+ if (PrevDecl->hasAttr<DLLExportAttr>()) {
+ Diag(AL.getLoc(), diag::warn_attr_dllexport_explicit_inst_def);
+ } else {
+ Diag(AL.getLoc(),
+ diag::warn_attr_dllexport_explicit_inst_def_mismatch);
+ Diag(PrevDecl->getLocation(), diag::note_prev_decl_missing_dllexport);
+ }
break;
}
}
}
+ if (TSK == TSK_ExplicitInstantiationDefinition && PrevDecl &&
+ !Context.getTargetInfo().getTriple().isWindowsGNUEnvironment() &&
+ llvm::none_of(Attr, [](const ParsedAttr &AL) {
+ return AL.getKind() == ParsedAttr::AT_DLLExport;
+ })) {
+ if (const auto *DEA = PrevDecl->getAttr<DLLExportOnDeclAttr>()) {
+ Diag(TemplateLoc, diag::warn_dllexport_on_decl_ignored);
+ Diag(DEA->getLoc(), diag::note_dllexport_on_decl);
+ }
+ }
+
if (CheckExplicitInstantiation(*this, ClassTemplate, TemplateNameLoc,
SS.isSet(), TSK))
return true;
diff --git a/clang/test/SemaCXX/dllexport-explicit-instantiation.cpp b/clang/test/SemaCXX/dllexport-explicit-instantiation.cpp
new file mode 100644
index 0000000000000..522e81d37976d
--- /dev/null
+++ b/clang/test/SemaCXX/dllexport-explicit-instantiation.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify=win32,win32-pedantic -DMS %s -Wdllexport-explicit-instantiation
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify=win32 -DMS %s -Wno-dllexport-explicit-instantiation
+// RUN: %clang_cc1 -triple x86_64-mingw32 -fsyntax-only -fms-extensions -verify=mingw,mingw-pedantic -DMS %s -Wdllexport-explicit-instantiation
+// RUN: %clang_cc1 -triple x86_64-mingw32 -fsyntax-only -fms-extensions -verify=mingw -DMS %s -Wno-dllexport-explicit-instantiation
+
+template <class>
+class S {};
+
+extern template class __declspec(dllexport) S<short>; // win32-pedantic-warning {{explicit instantiation declaration should not be 'dllexport'}} \
+ win32-pedantic-note {{attribute is here}}
+template class __declspec(dllexport) S<short>; // mingw-pedantic-warning {{'dllexport' attribute ignored on explicit instantiation definition}}
+
+extern template class __declspec(dllexport) S<int>; // win32-pedantic-warning {{explicit instantiation declaration should not be 'dllexport'}} \
+ win32-pedantic-note {{attribute is here}} \
+ win32-note {{'dllexport' attribute on the declaration is ignored}}
+template class S<int>; // win32-warning {{explicit instantiation definition is not exported without 'dllexport'}}
+
+extern template class S<long>; // mingw-note {{'dllexport' attribute is missing on previous declaration}}
+template class __declspec(dllexport) S<long>; // mingw-warning {{'dllexport' attribute ignored on explicit instantiation definition}}
diff --git a/clang/test/SemaCXX/dllexport.cpp b/clang/test/SemaCXX/dllexport.cpp
index 22d92c30954e8..1978e463a3f7a 100644
--- a/clang/test/SemaCXX/dllexport.cpp
+++ b/clang/test/SemaCXX/dllexport.cpp
@@ -462,6 +462,9 @@ template struct ExplicitlyInstantiatedTemplate<int>;
template <typename T> struct ExplicitlyExportInstantiatedTemplate { void func() {} };
template struct __declspec(dllexport) ExplicitlyExportInstantiatedTemplate<int>;
template <typename T> struct ExplicitlyExportDeclaredInstantiatedTemplate { void func() {} };
+#ifdef GNU
+// expected-note at +2 {{attribute is missing}}
+#endif
extern template struct ExplicitlyExportDeclaredInstantiatedTemplate<int>;
#if not defined(MS) && not defined (WI) && not defined(PS)
// expected-warning at +2{{'dllexport' attribute ignored on explicit instantiation definition}}
``````````
</details>
https://github.com/llvm/llvm-project/pull/133699
More information about the cfe-commits
mailing list