[PATCH] Downgrade "definition of dllimport static field" error to warning for class templates (PR19902)
Alp Toker
alp at nuanti.com
Mon Jun 2 20:40:19 PDT 2014
On 03/06/2014 05:37, Hans Wennborg wrote:
> Hi nrieck, rnk,
?
>
> This allows us to compile the following kind of code, which occurs in MSVC headers:
>
> template <typename> struct S {
> __declspec(dllimport) static int x;
> };
> template <typename T> int S<T>::x;
Based on your description it sounds like this isn't ill-formed but the
attribute is just ignored. Review inline...
> Please take a look.
>
> http://reviews.llvm.org/D3998
>
> Files:
> include/clang/Basic/DiagnosticSemaKinds.td
> lib/Sema/SemaDecl.cpp
> test/SemaCXX/dllimport.cpp
>
> D3998.10038.patch
>
>
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -2109,6 +2109,9 @@
> "definition of dllimport data">;
> def err_attribute_dllimport_static_field_definition : Error<
> "definition of dllimport static field not allowed">;
> +def warn_attribute_dllimport_static_field_definition : Warning<
> + "definition of dllimport static field not allowed">,
> + InGroup<DiagGroup<"dllimport-static-field-def">>;
If that's the case, just make this a warning, or at most, a DefaultError
warning (that will be ignored in system headers).
> def err_attribute_dll_member_of_dll_class : Error<
> "attribute %q0 cannot be applied to member of %q1 class">;
> def err_attribute_weakref_not_static : Error<
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -9061,10 +9061,17 @@
> if (const DLLImportAttr *IA = VD->getAttr<DLLImportAttr>()) {
> if (VD->isStaticDataMember() && VD->isOutOfLine() &&
> VD->isThisDeclarationADefinition()) {
> + // Definitions of dllimport class template static data members occurs
> + // in MSVC header files. Downgrade to a warning.
> + bool JustWarn = cast<CXXRecordDecl>(VD->getFirstDecl()->getDeclContext())
> + ->getDescribedClassTemplate();
> +
There is no need for that check and it doesn't look right.
> Diag(VD->getLocation(),
> - diag::err_attribute_dllimport_static_field_definition);
> + JustWarn ? diag::warn_attribute_dllimport_static_field_definition
> + : diag::err_attribute_dllimport_static_field_definition);
> Diag(IA->getLocation(), diag::note_attribute);
> - VD->setInvalidDecl();
> + if (!JustWarn)
> + VD->setInvalidDecl();
Assuming the code isn't ill-formed, the declaration should not be set
invalid at all.
Alp.
> }
> }
>
> Index: test/SemaCXX/dllimport.cpp
> ===================================================================
> --- test/SemaCXX/dllimport.cpp
> +++ test/SemaCXX/dllimport.cpp
> @@ -816,9 +816,9 @@
> template<typename T> inline void ImportClassTmplMembers<T>::staticInlineDef() {}
> template<typename T> void ImportClassTmplMembers<T>::staticInlineDecl() {}
>
> -template<typename T> int ImportClassTmplMembers<T>::StaticFieldDef; // expected-error{{definition of dllimport static field not allowed}}
> -template<typename T> const int ImportClassTmplMembers<T>::StaticConstFieldDef = 1; // expected-error{{definition of dllimport static field not allowed}}
> -template<typename T> constexpr int ImportClassTmplMembers<T>::ConstexprFieldDef; // expected-error{{definition of dllimport static field not allowed}}
> +template<typename T> int ImportClassTmplMembers<T>::StaticFieldDef; // expected-warning{{definition of dllimport static field not allowed}}
> +template<typename T> const int ImportClassTmplMembers<T>::StaticConstFieldDef = 1; // expected-warning{{definition of dllimport static field not allowed}}
> +template<typename T> constexpr int ImportClassTmplMembers<T>::ConstexprFieldDef; // expected-warning{{definition of dllimport static field not allowed}}
>
>
> // Redeclarations cannot add dllimport.
> @@ -853,13 +853,13 @@
> template<typename T> __declspec(dllimport) void CTMR<T>::staticInlineDecl() {} // expected-error{{redeclaration of 'CTMR::staticInlineDecl' cannot add 'dllimport' attribute}}
>
> template<typename T> __declspec(dllimport) int CTMR<T>::StaticField = 1; // expected-error{{redeclaration of 'CTMR::StaticField' cannot add 'dllimport' attribute}}
> - // expected-error at -1{{definition of dllimport static field not allowed}}
> + // expected-warning at -1{{definition of dllimport static field not allowed}}
> // expected-note at -2{{attribute is here}}
> template<typename T> __declspec(dllimport) const int CTMR<T>::StaticConstField = 1; // expected-error{{redeclaration of 'CTMR::StaticConstField' cannot add 'dllimport' attribute}}
> - // expected-error at -1{{definition of dllimport static field not allowed}}
> + // expected-warning at -1{{definition of dllimport static field not allowed}}
> // expected-note at -2{{attribute is here}}
> template<typename T> __declspec(dllimport) constexpr int CTMR<T>::ConstexprField; // expected-error{{redeclaration of 'CTMR::ConstexprField' cannot add 'dllimport' attribute}}
> - // expected-error at -1{{definition of dllimport static field not allowed}}
> + // expected-warning at -1{{definition of dllimport static field not allowed}}
> // expected-note at -2{{attribute is here}}
>
>
> @@ -901,9 +901,9 @@
> template<typename T> template<typename U> void ImportClsTmplMemTmpl<T>::staticInlineDecl() {}
>
> #if __has_feature(cxx_variable_templates)
> -template<typename T> template<typename U> int ImportClsTmplMemTmpl<T>::StaticFieldDef; // expected-error{{definition of dllimport static field not allowed}}
> -template<typename T> template<typename U> const int ImportClsTmplMemTmpl<T>::StaticConstFieldDef = 1; // expected-error{{definition of dllimport static field not allowed}}
> -template<typename T> template<typename U> constexpr int ImportClsTmplMemTmpl<T>::ConstexprFieldDef; // expected-error{{definition of dllimport static field not allowed}}
> +template<typename T> template<typename U> int ImportClsTmplMemTmpl<T>::StaticFieldDef; // expected-warning{{definition of dllimport static field not allowed}}
> +template<typename T> template<typename U> const int ImportClsTmplMemTmpl<T>::StaticConstFieldDef = 1; // expected-warning{{definition of dllimport static field not allowed}}
> +template<typename T> template<typename U> constexpr int ImportClsTmplMemTmpl<T>::ConstexprFieldDef; // expected-warning{{definition of dllimport static field not allowed}}
> #endif // __has_feature(cxx_variable_templates)
>
>
> @@ -935,13 +935,13 @@
>
> #if __has_feature(cxx_variable_templates)
> template<typename T> template<typename U> __declspec(dllimport) int CTMTR<T>::StaticField = 1; // expected-error{{redeclaration of 'CTMTR::StaticField' cannot add 'dllimport' attribute}}
> - // expected-error at -1{{definition of dllimport static field not allowed}}
> + // expected-warning at -1{{definition of dllimport static field not allowed}}
> // expected-note at -2{{attribute is here}}
> template<typename T> template<typename U> __declspec(dllimport) const int CTMTR<T>::StaticConstField = 1; // expected-error{{redeclaration of 'CTMTR::StaticConstField' cannot add 'dllimport' attribute}}
> - // expected-error at -1{{definition of dllimport static field not allowed}}
> + // expected-warning at -1{{definition of dllimport static field not allowed}}
> // expected-note at -2{{attribute is here}}
> template<typename T> template<typename U> __declspec(dllimport) constexpr int CTMTR<T>::ConstexprField; // expected-error{{redeclaration of 'CTMTR::ConstexprField' cannot add 'dllimport' attribute}}
> - // expected-error at -1{{definition of dllimport static field not allowed}}
> + // expected-warning at -1{{definition of dllimport static field not allowed}}
> // expected-note at -2{{attribute is here}}
> #endif // __has_feature(cxx_variable_templates)
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
More information about the cfe-commits
mailing list