<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jun 2, 2014 at 8:40 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
On 03/06/2014 05:37, Hans Wennborg wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Hi nrieck, rnk,<br>
</blockquote><div class="">
<br>
?<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
This allows us to compile the following kind of code, which occurs in MSVC headers:<br>
<br>
template <typename> struct S {<br>
__declspec(dllimport) static int x;<br>
};<br>
template <typename T> int S<T>::x;<br>
</blockquote>
<br></div>
Based on your description it sounds like this isn't ill-formed but the attribute is just ignored. Review inline...<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">
Please take a look.<br>
<br>
<a href="http://reviews.llvm.org/D3998" target="_blank">http://reviews.llvm.org/D3998</a><br>
<br>
Files:<br>
include/clang/Basic/<u></u>DiagnosticSemaKinds.td<br>
lib/Sema/SemaDecl.cpp<br>
test/SemaCXX/dllimport.cpp<br>
<br></div>
D3998.10038.patch<br>
<br>
<br>
Index: include/clang/Basic/<u></u>DiagnosticSemaKinds.td<br>
==============================<u></u>==============================<u></u>=======<br>
--- include/clang/Basic/<u></u>DiagnosticSemaKinds.td<br>
+++ include/clang/Basic/<u></u>DiagnosticSemaKinds.td<br>
@@ -2109,6 +2109,9 @@<br>
"definition of dllimport data">;<br>
def err_attribute_dllimport_<u></u>static_field_definition : Error<<br>
"definition of dllimport static field not allowed">;<br>
+def warn_attribute_dllimport_<u></u>static_field_definition : Warning<<br>
+ "definition of dllimport static field not allowed">,<br>
+ InGroup<DiagGroup<"dllimport-<u></u>static-field-def">>;<br>
</blockquote>
<br>
If that's the case, just make this a warning, or at most, a DefaultError warning (that will be ignored in system headers).<br></blockquote><div><br></div><div>The non-template case should definitely remain an error.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
def err_attribute_dll_member_of_<u></u>dll_class : Error<<br>
"attribute %q0 cannot be applied to member of %q1 class">;<br>
def err_attribute_weakref_not_<u></u>static : Error<<br>
Index: lib/Sema/SemaDecl.cpp<br>
==============================<u></u>==============================<u></u>=======<br>
--- lib/Sema/SemaDecl.cpp<br>
+++ lib/Sema/SemaDecl.cpp<br>
@@ -9061,10 +9061,17 @@<br>
if (const DLLImportAttr *IA = VD->getAttr<DLLImportAttr>()) {<br>
if (VD->isStaticDataMember() && VD->isOutOfLine() &&<br>
VD-><u></u>isThisDeclarationADefinition()<u></u>) {<br>
+ // Definitions of dllimport class template static data members occurs<br>
+ // in MSVC header files. Downgrade to a warning.<br>
+ bool JustWarn = cast<CXXRecordDecl>(VD-><u></u>getFirstDecl()-><u></u>getDeclContext())<br>
+ ->getDescribedClassTemplate();<br>
+<br>
</blockquote>
<br>
There is no need for that check and it doesn't look right.<br></blockquote><div><br></div><div>The way I think about these definitions of static data members of not-fully-specialized class templates is that they're like inline functions. I don't see any reason why you can't provide a definition out of line while still importing the definition from another DLL. It should get available_externally linkage. If it's not const, it's not very useful for optimization purposes, but it seems semantically consistent.</div>
<div><br></div><div>What about the check do you think is wrong? The one case I'd want to make sure we still error on is the out-of-line definition of a static data member of a fully-specialized class template:</div><div>
<div>template <typename T> struct S { static int x; };</div><div>template <typename T> int S<T>::x = sizeof(T);</div><div>template <> struct __declspec(dllimport) S<int> { static int x; };</div>
<div>int S<int>::x = -1; // should error, only the dll should have this definition.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Diag(VD->getLocation(),<br>
- diag::err_attribute_dllimport_<u></u>static_field_definition);<br>
+ JustWarn ? diag::warn_attribute_<u></u>dllimport_static_field_<u></u>definition<br>
+ : diag::err_attribute_dllimport_<u></u>static_field_definition);<br>
Diag(IA->getLocation(), diag::note_attribute);<br>
- VD->setInvalidDecl();<br>
+ if (!JustWarn)<br>
+ VD->setInvalidDecl();<br>
</blockquote>
<br>
Assuming the code isn't ill-formed, the declaration should not be set invalid at all.<br>
<br>
Alp.<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
}<br>
}<br>
Index: test/SemaCXX/dllimport.cpp<br>
==============================<u></u>==============================<u></u>=======<br>
--- test/SemaCXX/dllimport.cpp<br>
+++ test/SemaCXX/dllimport.cpp<br>
@@ -816,9 +816,9 @@<br>
template<typename T> inline void ImportClassTmplMembers<T>::<u></u>staticInlineDef() {}<br>
template<typename T> void ImportClassTmplMembers<T>::<u></u>staticInlineDecl() {}<br>
-template<typename T> int ImportClassTmplMembers<T>::<u></u>StaticFieldDef; // expected-error{{definition of dllimport static field not allowed}}<br>
-template<typename T> const int ImportClassTmplMembers<T>::<u></u>StaticConstFieldDef = 1; // expected-error{{definition of dllimport static field not allowed}}<br>
-template<typename T> constexpr int ImportClassTmplMembers<T>::<u></u>ConstexprFieldDef; // expected-error{{definition of dllimport static field not allowed}}<br>
+template<typename T> int ImportClassTmplMembers<T>::<u></u>StaticFieldDef; // expected-warning{{definition of dllimport static field not allowed}}<br>
+template<typename T> const int ImportClassTmplMembers<T>::<u></u>StaticConstFieldDef = 1; // expected-warning{{definition of dllimport static field not allowed}}<br>
+template<typename T> constexpr int ImportClassTmplMembers<T>::<u></u>ConstexprFieldDef; // expected-warning{{definition of dllimport static field not allowed}}<br>
// Redeclarations cannot add dllimport.<br>
@@ -853,13 +853,13 @@<br>
template<typename T> __declspec(dllimport) void CTMR<T>::staticInlineDecl() {} // expected-error{{redeclaration of 'CTMR::staticInlineDecl' cannot add 'dllimport' attribute}}<br>
template<typename T> __declspec(dllimport) int CTMR<T>::StaticField = 1; // expected-error{{redeclaration of 'CTMR::StaticField' cannot add 'dllimport' attribute}}<br>
- // expected-error@-1{{definition of dllimport static field not allowed}}<br>
+ // expected-warning@-1{{<u></u>definition of dllimport static field not allowed}}<br>
// expected-note@-2{{attribute is here}}<br>
template<typename T> __declspec(dllimport) const int CTMR<T>::StaticConstField = 1; // expected-error{{redeclaration of 'CTMR::StaticConstField' cannot add 'dllimport' attribute}}<br>
- // expected-error@-1{{definition of dllimport static field not allowed}}<br>
+ // expected-warning@-1{{<u></u>definition of dllimport static field not allowed}}<br>
// expected-note@-2{{attribute is here}}<br>
template<typename T> __declspec(dllimport) constexpr int CTMR<T>::ConstexprField; // expected-error{{redeclaration of 'CTMR::ConstexprField' cannot add 'dllimport' attribute}}<br>
- // expected-error@-1{{definition of dllimport static field not allowed}}<br>
+ // expected-warning@-1{{<u></u>definition of dllimport static field not allowed}}<br>
// expected-note@-2{{attribute is here}}<br>
@@ -901,9 +901,9 @@<br>
template<typename T> template<typename U> void ImportClsTmplMemTmpl<T>::<u></u>staticInlineDecl() {}<br>
#if __has_feature(cxx_variable_<u></u>templates)<br>
-template<typename T> template<typename U> int ImportClsTmplMemTmpl<T>::<u></u>StaticFieldDef; // expected-error{{definition of dllimport static field not allowed}}<br>
-template<typename T> template<typename U> const int ImportClsTmplMemTmpl<T>::<u></u>StaticConstFieldDef = 1; // expected-error{{definition of dllimport static field not allowed}}<br>
-template<typename T> template<typename U> constexpr int ImportClsTmplMemTmpl<T>::<u></u>ConstexprFieldDef; // expected-error{{definition of dllimport static field not allowed}}<br>
+template<typename T> template<typename U> int ImportClsTmplMemTmpl<T>::<u></u>StaticFieldDef; // expected-warning{{definition of dllimport static field not allowed}}<br>
+template<typename T> template<typename U> const int ImportClsTmplMemTmpl<T>::<u></u>StaticConstFieldDef = 1; // expected-warning{{definition of dllimport static field not allowed}}<br>
+template<typename T> template<typename U> constexpr int ImportClsTmplMemTmpl<T>::<u></u>ConstexprFieldDef; // expected-warning{{definition of dllimport static field not allowed}}<br>
#endif // __has_feature(cxx_variable_<u></u>templates)<br>
@@ -935,13 +935,13 @@<br>
#if __has_feature(cxx_variable_<u></u>templates)<br>
template<typename T> template<typename U> __declspec(dllimport) int CTMTR<T>::StaticField = 1; // expected-error{{redeclaration of 'CTMTR::StaticField' cannot add 'dllimport' attribute}}<br>
- // expected-error@-1{{definition of dllimport static field not allowed}}<br>
+ // expected-warning@-1{{<u></u>definition of dllimport static field not allowed}}<br>
// expected-note@-2{{attribute is here}}<br>
template<typename T> template<typename U> __declspec(dllimport) const int CTMTR<T>::StaticConstField = 1; // expected-error{{redeclaration of 'CTMTR::StaticConstField' cannot add 'dllimport' attribute}}<br>
- // expected-error@-1{{definition of dllimport static field not allowed}}<br>
+ // expected-warning@-1{{<u></u>definition of dllimport static field not allowed}}<br>
// expected-note@-2{{attribute is here}}<br>
template<typename T> template<typename U> __declspec(dllimport) constexpr int CTMTR<T>::ConstexprField; // expected-error{{redeclaration of 'CTMTR::ConstexprField' cannot add 'dllimport' attribute}}<br>
- // expected-error@-1{{definition of dllimport static field not allowed}}<br>
+ // expected-warning@-1{{<u></u>definition of dllimport static field not allowed}}<br>
// expected-note@-2{{attribute is here}}<br>
#endif // __has_feature(cxx_variable_<u></u>templates)<br>
<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><span class=""><font color="#888888"><br>
</font></span></blockquote><span class=""><font color="#888888">
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</font></span></blockquote></div><br></div></div>