[PATCH] Fix dllimport attribute handling on C++ static data members (PR c/88568)

[Jason Merrill]

On 3/5/19 3:14 PM, Jakub Jelinek wrote:
> On Thu, Jan 10, 2019 at 11:07:37AM +0100, Jakub Jelinek wrote:
>> 2019-01-10  Jakub Jelinek  <jakub@redhat.com>
>>
>> 	PR c/88568
>> 	* attribs.c (handle_dll_attribute): Clear TREE_STATIC after setting
>> 	DECL_EXTERNAL.
>>
>> 	* gcc.dg/pr88568.c: New test.
>>
>> --- gcc/attribs.c.jj	2019-01-05 12:06:12.055124090 +0100
>> +++ gcc/attribs.c	2019-01-07 12:57:09.739782281 +0100
>> @@ -1691,6 +1691,8 @@ handle_dll_attribute (tree * pnode, tree
>>   	     a function global scope, unless declared static.  */
>>   	  if (current_function_decl != NULL_TREE && !TREE_STATIC (node))
>>   	    TREE_PUBLIC (node) = 1;
>> +	  /* Clear TREE_STATIC because DECL_EXTERNAL is set.  */
>> +	  TREE_STATIC (node) = 0;
>>   	}
>>   
>>         if (*no_add_attrs == false)
> 
> The above change apparently broke handling of dllimport C++ static data
> members (on both trunk in 8.3), where the C++ FE requires that TREE_STATIC
> is set on the static data members, on the other side handles well the case
> when it is TREE_STATIC and DECL_EXTERNAL at the same time.

Yes, that flag combination seems entirely reasonable to me: it's a 
static-storage-duration variable that doesn't happen to be defined in 
this translation unit (yet).

In several cases the C++ front end leaves DECL_EXTERNAL set on things 
that have definitions until EOF, when we decide what actually needs to 
be emitted.  This is obsolete since the advent of cgraph, but I haven't 
found the time to rip it out.  It looks like we don't do that for 
namespace-scope variable templates, though, so they should be OK.

The patch is OK with me.

Jason