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@+2 {{attribute is missing}} +#endif extern template struct ExplicitlyExportDeclaredInstantiatedTemplate<int>; #if not defined(MS) && not defined (WI) && not defined(PS) // expected-warning@+2{{'dllexport' attribute ignored on explicit instantiation definition}} `````````` </details> https://github.com/llvm/llvm-project/pull/133699 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
