Message ID | 20230501205454.1627105-1-jason@redhat.com |
---|---|
State | New |
Headers | show |
Series | [pushed] c++: array DMI and member fn [PR109666] | expand |
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; }
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; > }
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; + } + + return push_to_top; +} + +/* Return from whatever maybe_push_to_top_level did. */ + +void +maybe_pop_from_top_level (bool push_to_top) +{ + if (push_to_top) + pop_from_top_level (); + else + pop_function_context (); +} + /* Push into the scope of the namespace NS, even if it is deeply nested within another namespace. */ diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 6df16fef0dd..3f1cf139bbd 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -26815,20 +26815,7 @@ instantiate_body (tree pattern, tree args, tree d, bool nested_p) if (current_function_decl) save_omp_privatization_clauses (omp_privatization_save); - 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; - } + bool push_to_top = maybe_push_to_top_level (d); mark_template_arguments_used (pattern, args); @@ -26942,10 +26929,7 @@ instantiate_body (tree pattern, tree args, tree d, bool nested_p) if (!nested_p) TI_PENDING_TEMPLATE_FLAG (DECL_TEMPLATE_INFO (d)) = 0; - if (push_to_top) - pop_from_top_level (); - else - pop_function_context (); + maybe_pop_from_top_level (push_to_top); if (current_function_decl) restore_omp_privatization_clauses (omp_privatization_save); diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-array2.C b/gcc/testsuite/g++.dg/cpp0x/nsdmi-array2.C new file mode 100644 index 00000000000..5ad60f56510 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-array2.C @@ -0,0 +1,15 @@ +// PR c++/109666 +// { dg-do compile { target c++11 } } + +struct Point { + int value_; +}; +template <int n> struct StaticVector { + static StaticVector create() { + StaticVector output; + return output; + } + Point _M_elems[n]{}; + +}; +void f() { StaticVector<3>::create(); }