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

Hans Wennborg hans at chromium.org
Tue Jun 3 17:25:41 PDT 2014


On Tue, Jun 3, 2014 at 5:09 PM, Nico Rieck <nico.rieck at gmail.com> wrote:
> On 04.06.2014 01:18, Hans Wennborg wrote:
>>> 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.
>
> But curiously they do accept it if S<int>::x is explicitly instantiated.
> It's unfortunate that they yet again botched their headers. Not sure if
> you were referring to <locale> earlier, but the same issue exists there
> (see PR13450).
>
> With which MinGW version did you test? Mine (4.9.0) just drops the
> dllimport attribute but this doesn't mean much as they don't seem to
> implement any inline semantics for dll attributes.

Looks like I have 4.6.2. Time to upgrade, it seems.

>>> 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.
>
> Being more permissive here but making the diagnostic configurable to get
> the strict behavior is a nice trade-off IMO.
>
>
> On 04.06.2014 01:36, Alp Toker wrote:>
>> Name this IsValid. The fact it causes a warning is tangential.
>
> Then I'd name it for what it actually represents: whether the variable
> is a member of a class or a class template.

That sounds good to me.

Thanks everyone for reviewing!

 - Hans



More information about the cfe-commits mailing list