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

[Patrick Palka <ppalka@redhat.com>]

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.
	(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;
 }
-- 
2.43.0.121.g624eb90fa8