[PATCH] Downgrade "definition of dllimport static field" error to warning for class templates (PR19902)
hans at chromium.org
Tue Jun 3 16:18:44 PDT 2014
On Tue, Jun 3, 2014 at 3:36 PM, Alp Toker <alp at nuanti.com> wrote:
> On 04/06/2014 01:10, Reid Kleckner wrote:
>> On Mon, Jun 2, 2014 at 8:40 PM, Alp Toker <alp at nuanti.com
>> If that's the case, just make this a warning, or at most, a
>> DefaultError warning (that will be ignored in system headers).
>> The non-template case should definitely remain an error.
> What does MSVC do? Reject, accept or warn?
They reject it.
>> 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.
> If it's perfectly OK and semantically consistent to accept this then why
> emit a warning?
For compatibility, I guess. MSVC rejects the code if S is
instantiated. MinGW errors, but allows it with a warning when using
-fpermissive. A warning which indicates that the code is in a dodgy
corner of dllimport land seems valuable.
In any case, the warning certainly shouldn't say "is not allowed" if
we actually allow it :)
>> What about the check do you think is wrong?
> We'd normally check linkage/inlining or use some other existing predicate
> for a check like this instead of poking at the decl for a specific special
We can't check the linkage; all dllimport static fields will be
available_externally. I don't know of any nicer way to check for this
> Also note the patch was described as a system header workaround, while
> you're talking about accepting it as fully valid user code. The correct way
> to implement this patch depends on which of the two is preferable -- so that
> needs to be decided on first.
Yes, I think Reid is suggesting accepting it as valid user code and
I'm fine with that.
More information about the cfe-commits