Re: [C++ PATCH] Fix thread_local initialization (PR c++/60702)

[Jason Merrill <jason at redhat dot com>]

On 3/21/19 6:55 PM, Jakub Jelinek wrote:
> On Thu, Mar 21, 2019 at 05:03:11PM -0400, Jason Merrill wrote:
> > > --- gcc/cp/semantics.c.jj	2019-03-14 09:14:16.718012031 +0100
> > > +++ gcc/cp/semantics.c	2019-03-15 16:53:14.270384477 +0100
> > > @@ -2135,6 +2135,17 @@ finish_qualified_id_expr (tree qualifyin
> > >    	expr = build_qualified_name (TREE_TYPE (expr),
> > >    				     qualifying_class, expr,
> > >    				     template_p);
> > > +      else if (VAR_P (expr)
> > > +	       && !processing_template_decl
> > > +	       && !cp_unevaluated_operand
> > > +	       && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
> > > +	       && CP_DECL_THREAD_LOCAL_P (expr))
> > > +	{
> > > +	  if (tree wrap = get_tls_wrapper_fn (expr))
> > > +	    /* Replace an evaluated use of the thread_local variable with
> > > +	       a call to its wrapper.  */
> > > +	    expr = build_cxx_call (wrap, 0, NULL, tf_warning_or_error);
> > > +	}
> >
> > Let's factor this pattern out into another function.  And we don't need to
> > check TREE_STATIC || DECL_EXTERNAL anymore.
>
> So, do you want say:
> tree
> maybe_get_tls_wrapper_fn (tree expr)
> {
>    if (VAR_P (expr)
>        && !processing_template_decl
>        && !cp_unevaluated_operand
>        && CP_DECL_THREAD_LOCAL_P (expr))
>      return get_tls_wrapper_fn (expr);
>    return NULL;
> }
> and use it like:
>    else if (tree wrap = maybe_get_tls_wrapper_fn (expr))
>      /* Replace an evaluated use of the thread_local variable with
>         a call to its wrapper.  */
>      expr = build_cxx_call (wrap, 0, NULL, tf_warning_or_error);
>    else
>      ...
> or even move the build_cxx_call into that function (then I don't know how to
> call it, but what's worse, many of the spots where it is used want to bypass
> further processing if we've replaced the TLS var with the call, so if the
> build_cxx_call was in there, we'd need a function that returns two values,
> so perhaps returning a bool and take the expr as reference, but I believe
> current coding guidelines are against doing something like that.

How about

else if (tree wrap = maybe_call_tls_wrapper_fn (expr))
  expr = wrap;

?

> For the TREE_STATIC || DECL_EXTERNAL, I assume it is safe to drop it in the
> current two spots (pt.c and semantics.c) too

Yes.  That check was from back before the addition of 
CP_DECL_THREAD_LOCAL_P, which doesn't need this guard.  Nor does 
DECL_THREAD_LOCAL_P any more, it seems.

Jason