Re: [pushed] c++: array DMI and member fn [PR109666]

[Jason Merrill <jason@redhat.com>]

On 12/19/23 14:08, Patrick Palka wrote:
> On Mon, 1 May 2023, Jason Merrill wrote:
> 
>> Tested x86_64-pc-linux-gnu, applying to trunk.
>>
>> Patrick, can you verify that this resolves 109506 and add whatever testcase(s)
>> seem appropriate from that PR?
>>
>> -- 8< --
>>
>> Here it turns out I also needed to adjust cfun when stepping out of the
>> member function to instantiate the DMI.  But instead of adding that tweak,
>> let's unify with instantiate_body and just push_to_top_level instead of
>> trying to do the minimum subset of it.  There was no measurable change in
>> compile time on stdc++.h.
>>
>> This should also resolve 109506 without yet another tweak.
>>
>> 	PR c++/109666
>>
>> gcc/cp/ChangeLog:
>>
>> 	* name-lookup.cc (maybe_push_to_top_level)
>> 	(maybe_pop_from_top_level): Split out...
>> 	* pt.cc (instantiate_body): ...from here.
>> 	* init.cc (maybe_instantiate_nsdmi_init): Use them.
>> 	* name-lookup.h: Declare them..
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* g++.dg/cpp0x/nsdmi-array2.C: New test.
>> ---
>>   gcc/cp/name-lookup.h                      |  2 ++
>>   gcc/cp/init.cc                            | 25 ++-------------
>>   gcc/cp/name-lookup.cc                     | 37 +++++++++++++++++++++++
>>   gcc/cp/pt.cc                              | 20 ++----------
>>   gcc/testsuite/g++.dg/cpp0x/nsdmi-array2.C | 15 +++++++++
>>   5 files changed, 58 insertions(+), 41 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-array2.C
>>
>> diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
>> index c234cd44356..b3e708561d8 100644
>> --- a/gcc/cp/name-lookup.h
>> +++ b/gcc/cp/name-lookup.h
>> @@ -466,6 +466,8 @@ extern void push_nested_namespace (tree);
>>   extern void pop_nested_namespace (tree);
>>   extern void push_to_top_level (void);
>>   extern void pop_from_top_level (void);
>> +extern bool maybe_push_to_top_level (tree);
>> +extern void maybe_pop_from_top_level (bool);
>>   extern void push_using_decl_bindings (tree, tree);
>>   
>>   /* Lower level interface for modules. */
>> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
>> index 1dd24e30d7c..0b35e1092e9 100644
>> --- a/gcc/cp/init.cc
>> +++ b/gcc/cp/init.cc
>> @@ -597,32 +597,14 @@ maybe_instantiate_nsdmi_init (tree member, tsubst_flags_t complain)
>>   	  bool pushed = false;
>>   	  tree ctx = type_context_for_name_lookup (member);
>>   
>> -	  processing_template_decl_sentinel ptds (/*reset*/false);
>> +	  bool push_to_top = maybe_push_to_top_level (member);
>>   	  if (!currently_open_class (ctx))
>>   	    {
>> -	      if (!LOCAL_CLASS_P (ctx))
>> -		push_to_top_level ();
>> -	      else
>> -		/* push_to_top_level would lose the necessary function context,
>> -		   just reset processing_template_decl.  */
>> -		processing_template_decl = 0;
>>   	      push_nested_class (ctx);
>>   	      push_deferring_access_checks (dk_no_deferred);
>>   	      pushed = true;
>>   	    }
>>   
>> -	  /* If we didn't push_to_top_level, still step out of constructor
>> -	     scope so build_base_path doesn't try to use its __in_chrg.  */
>> -	  tree cfd = current_function_decl;
>> -	  auto cbl = current_binding_level;
>> -	  if (at_function_scope_p ())
>> -	    {
>> -	      current_function_decl
>> -		= decl_function_context (current_function_decl);
>> -	      while (current_binding_level->kind != sk_class)
>> -		current_binding_level = current_binding_level->level_chain;
>> -	    }
>> -
>>   	  inject_this_parameter (ctx, TYPE_UNQUALIFIED);
>>   
>>   	  start_lambda_scope (member);
>> @@ -639,15 +621,12 @@ maybe_instantiate_nsdmi_init (tree member, tsubst_flags_t complain)
>>   	  if (init != error_mark_node)
>>   	    DECL_INITIAL (member) = init;
>>   
>> -	  current_function_decl = cfd;
>> -	  current_binding_level = cbl;
>>   	  if (pushed)
>>   	    {
>>   	      pop_deferring_access_checks ();
>>   	      pop_nested_class ();
>> -	      if (!LOCAL_CLASS_P (ctx))
>> -		pop_from_top_level ();
>>   	    }
>> +	  maybe_pop_from_top_level (push_to_top);
>>   
>>   	  input_location = sloc;
>>   	}
>> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
>> index 477cddd7543..7c61bc3bf61 100644
>> --- a/gcc/cp/name-lookup.cc
>> +++ b/gcc/cp/name-lookup.cc
>> @@ -8236,6 +8236,43 @@ pop_from_top_level (void)
>>     free_saved_scope = s;
>>   }
>>   
>> +/* Like push_to_top_level, but not if D is function-local.  Returns whether we
>> +   did push to top.  */
>> +
>> +bool
>> +maybe_push_to_top_level (tree d)
>> +{
>> +  /* Push if D isn't function-local, or is a lambda function, for which name
>> +     resolution is already done.  */
>> +  bool push_to_top
>> +    = !(current_function_decl
>> +	&& !LAMBDA_FUNCTION_P (d)
>> +	&& decl_function_context (d) == current_function_decl);
>> +
>> +  if (push_to_top)
>> +    push_to_top_level ();
>> +  else
>> +    {
>> +      gcc_assert (!processing_template_decl);
>> +      push_function_context ();
>> +      cp_unevaluated_operand = 0;
>> +      c_inhibit_evaluation_warnings = 0;
> 
> I think we need to restore these flags in maybe_pop_from_top_level, like
> so perhaps?  Bootstrap and regtest running on x86_64-pc-linux-gnu.
> 
> -- >8 --
> 
> Subject: [PATCH] c++: missing state restoration in maybe_pop_from_top_level
> 
> gcc/cp/ChangeLog:
> 
> 	* name-lookup.cc (struct maybe_push_state_t): Define.
> 	(maybe_push_state_stack): Define.

I might call these local_state_t/local_state_stack since that's the case 
they're used for?  OK either way.

> 	(maybe_push_to_top_level): Use them.
> 	(maybe_pop_from_top_level): Likewise.
> 	* pt.cc (instantiate_decl): Remove dead code for saving/restoring
> 	cp_unevaluated_operand and c_inhibit_evaluation_warnings.
> ---
>   gcc/cp/name-lookup.cc | 37 ++++++++++++++++++++++++++++++++++---
>   gcc/cp/pt.cc          |  4 ----
>   2 files changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index 09dc6ef3e5a..aaedfaa45b3 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -8553,6 +8553,35 @@ pop_from_top_level (void)
>     free_saved_scope = s;
>   }
>   
> +/* Helper class for saving/restoring relevant global flags in the
> +   function-local case of maybe_push_to_top_level.  */
> +
> +struct maybe_push_state_t
> +{
> +  int cp_unevaluated_operand;
> +  int c_inhibit_evaluation_warnings;
> +
> +  static maybe_push_state_t
> +  save_and_clear ()
> +  {
> +    maybe_push_state_t s;
> +    s.cp_unevaluated_operand = ::cp_unevaluated_operand;
> +    ::cp_unevaluated_operand = 0;
> +    s.c_inhibit_evaluation_warnings = ::c_inhibit_evaluation_warnings;
> +    ::c_inhibit_evaluation_warnings = 0;
> +    return s;
> +  }
> +
> +  void
> +  restore () const
> +  {
> +    ::cp_unevaluated_operand = this->cp_unevaluated_operand;
> +    ::c_inhibit_evaluation_warnings = this->c_inhibit_evaluation_warnings;
> +  }
> +};
> +
> +static vec<maybe_push_state_t> maybe_push_state_stack;
> +
>   /* Like push_to_top_level, but not if D is function-local.  Returns whether we
>      did push to top.  */
>   
> @@ -8572,8 +8601,7 @@ maybe_push_to_top_level (tree d)
>       {
>         gcc_assert (!processing_template_decl);
>         push_function_context ();
> -      cp_unevaluated_operand = 0;
> -      c_inhibit_evaluation_warnings = 0;
> +      maybe_push_state_stack.safe_push (maybe_push_state_t::save_and_clear ());
>       }
>   
>     return push_to_top;
> @@ -8587,7 +8615,10 @@ maybe_pop_from_top_level (bool push_to_top)
>     if (push_to_top)
>       pop_from_top_level ();
>     else
> -    pop_function_context ();
> +    {
> +      maybe_push_state_stack.pop ().restore ();
> +      pop_function_context ();
> +    }
>   }
>   
>   /* Push into the scope of the namespace NS, even if it is deeply
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index a82d7ae93aa..74da35596a5 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -26833,8 +26833,6 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p)
>     tree gen_tmpl;
>     bool pattern_defined;
>     location_t saved_loc = input_location;
> -  int saved_unevaluated_operand = cp_unevaluated_operand;
> -  int saved_inhibit_evaluation_warnings = c_inhibit_evaluation_warnings;
>     bool external_p;
>     bool deleted_p;
>   
> @@ -27071,8 +27069,6 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p)
>     pop_deferring_access_checks ();
>     pop_tinst_level ();
>     input_location = saved_loc;
> -  cp_unevaluated_operand = saved_unevaluated_operand;
> -  c_inhibit_evaluation_warnings = saved_inhibit_evaluation_warnings;
>   
>     return d;
>   }