[PATCH] Downgrade "definition of dllimport static field" error to warning for class templates (PR19902)

Hans Wennborg 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
> case.

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
case.

> 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.

Thanks,
Hans



More information about the cfe-commits mailing list