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

[Jason Merrill <jason at redhat dot com>]

On 3/5/19 4:03 PM, Jason Merrill wrote:
> 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).

Or, more specifically, that we aren't (yet) planning to emit in this 
translation unit even if we have a definition.

Jason