[PATCH] c++: ICE with noexcept in class in member function [PR96623]

Marek Polacek polacek@redhat.com
Fri Jan 22 22:03:16 GMT 2021


On Fri, Jan 22, 2021 at 04:44:42PM -0500, Jason Merrill wrote:
> On 1/22/21 4:01 PM, Marek Polacek wrote:
> > On Thu, Jan 21, 2021 at 09:47:35PM -0500, Jason Merrill via Gcc-patches wrote:
> > > On 1/21/21 5:45 PM, Marek Polacek wrote:
> > > > I discovered very strange code in inject_parm_decls:
> > > > 
> > > >      if (args && is_this_parameter (args))
> > > >        {
> > > >          gcc_checking_assert (current_class_ptr == NULL_TREE);
> > > >          current_class_ptr = NULL_TREE;
> > > > 
> > > > We are tripping up on the assert because when we call inject_parm_decls,
> > > > current_class_ptr is set to 'A'.  It was set by inject_this_parameter
> > > > after we've parsed the parameter-declaration-clause of the member
> > > > function foo.
> > > 
> > > But then it should be restored (to null) by the ccp = save_ccp a few lines
> > > later.
> > 
> > Indeed.  I glossed over that.  :(
> > > > It seems correct to set ccp/ccr to A::B when we're
> > > > late parsing the noexcept-specifiers of bar* functions in B, so that
> > > > this-> does the right thing.
> > > 
> > > Agreed.
> > > 
> > > > Since inject_parm_decls can mess with
> > > > ccp/ccr, I think best if we properly restore it after the late parsing
> > > > of noexcept-specifiers.
> > > 
> > > pop_injected_parms clears them, which is restoring them if we keep the
> > > assert.
> > > 
> > > > It should also work to clear ccp before calling inject_parm_decls, and
> > > > removing the assignment following the assert, should the assert stay.
> > > 
> > > But why is it non-null before parsing the unparsed_noexcepts?
> > 
> > Now that I've taken another look I see that ccp/ccr are being set
> > in start_preparsed_function (around line 16566), when we parse the
> > body of foo in cp_parser_late_parsing_for_member.  Therefore it's
> > sort of expected that it's still set when we get to B in foo.
> 
> Ah!  Yes, it's set because we're parsing a local class of A::foo.  This all
> makes more sense to me now.  So:
> 
> > -      gcc_checking_assert (current_class_ptr == NULL_TREE);
> 
> Let's keep the assert, and...
> 
> > +      tree save_ccp = current_class_ptr;
> > +      tree save_ccr = current_class_ref;
> 
> ...clear cc* after saving so the assert succeeds
> 
> >        vec_safe_truncate (unparsed_noexcepts, 0);
> > +      current_class_ptr = save_ccp;
> > +      current_class_ref = save_ccr;
> 
> No point restoring the original values (for A::foo) here, it won't be usable
> for any DMI in a local class of A.
> 
> OK with those changes.

Thanks a lot.  Here's what I'm going to check in after another round o
testing.

-- >8 --
I discovered very strange code in inject_parm_decls:

   if (args && is_this_parameter (args))
     {
       gcc_checking_assert (current_class_ptr == NULL_TREE);
       current_class_ptr = NULL_TREE;

We are tripping up on the assert because when we call inject_parm_decls,
current_class_ptr is set to 'A'.  It was set by inject_this_parameter
after we've parsed the parameter-declaration-clause of the member
function foo.  It seems correct to set ccp/ccr to A::B when we're
late parsing the noexcept-specifiers of bar* functions in B, so that
this-> does the right thing.  Since inject_parm_decls doesn't expect
to see non-null ccp/ccr, reset it before calling inject_parm_decls.

gcc/cp/ChangeLog:

	PR c++/96623
	* parser.c (inject_parm_decls): Remove a redundant assignment.
	(cp_parser_class_specifier_1): Clear current_class_{ptr,ref}
	before calling inject_parm_decls.

gcc/testsuite/ChangeLog:

	PR c++/96623
	* g++.dg/cpp0x/noexcept64.C: New test.
---
 gcc/cp/parser.c                         | 10 +++++-----
 gcc/testsuite/g++.dg/cpp0x/noexcept64.C | 24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept64.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 4b2bca3fd11..e0208d02bdc 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -24710,7 +24710,6 @@ inject_parm_decls (tree decl)
   if (args && is_this_parameter (args))
     {
       gcc_checking_assert (current_class_ptr == NULL_TREE);
-      current_class_ptr = NULL_TREE;
       current_class_ref = cp_build_fold_indirect_ref (args);
       current_class_ptr = args;
     }
@@ -24967,7 +24966,6 @@ cp_parser_class_specifier_1 (cp_parser* parser)
       tree pushed_scope = NULL_TREE;
       unsigned ix;
       cp_default_arg_entry *e;
-      tree save_ccp, save_ccr;
 
       if (!type_definition_ok_p || any_erroneous_template_args_p (type))
 	{
@@ -25012,6 +25010,8 @@ cp_parser_class_specifier_1 (cp_parser* parser)
       /* If there are noexcept-specifiers that have not yet been processed,
 	 take care of them now.  Do this before processing NSDMIs as they
 	 may depend on noexcept-specifiers already having been processed.  */
+      tree save_ccp = current_class_ptr;
+      tree save_ccr = current_class_ref;
       FOR_EACH_VEC_SAFE_ELT (unparsed_noexcepts, ix, decl)
 	{
 	  tree ctx = DECL_CONTEXT (decl);
@@ -25029,7 +25029,9 @@ cp_parser_class_specifier_1 (cp_parser* parser)
 	  /* Make sure that any template parameters are in scope.  */
 	  maybe_begin_member_template_processing (decl);
 
-	  /* Make sure that any member-function parameters are in scope.  */
+	  /* Make sure that any member-function parameters are in scope.
+	     This function doesn't expect ccp to be set.  */
+	  current_class_ptr = current_class_ref = NULL_TREE;
 	  inject_parm_decls (decl);
 
 	  /* 'this' is not allowed in static member functions.  */
@@ -25065,8 +25067,6 @@ cp_parser_class_specifier_1 (cp_parser* parser)
       vec_safe_truncate (unparsed_noexcepts, 0);
 
       /* Now parse any NSDMIs.  */
-      save_ccp = current_class_ptr;
-      save_ccr = current_class_ref;
       FOR_EACH_VEC_SAFE_ELT (unparsed_nsdmis, ix, decl)
 	{
 	  if (class_type != DECL_CONTEXT (decl))
diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept64.C b/gcc/testsuite/g++.dg/cpp0x/noexcept64.C
new file mode 100644
index 00000000000..8b7303cd8a1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/noexcept64.C
@@ -0,0 +1,24 @@
+// PR c++/96623
+// { dg-do compile { target c++11 } }
+
+constexpr int x = 0;
+struct A {
+  int a1;
+  void foo (int p) {
+    int foovar;
+    struct B {
+      int b1;
+      void bar1 () noexcept(x);
+      void bar2 () noexcept(noexcept(this->b1));
+      void bar3 () noexcept(noexcept(this->b2));
+      void bar4 () noexcept(noexcept(a1));
+      void bar5 () noexcept(noexcept(a2));
+      void bar6 () noexcept(noexcept(b1));
+      void bar7 () noexcept(noexcept(b2));
+      void bar8 () noexcept(noexcept(foovar));
+      void bar9 () noexcept(noexcept(p));
+      int b2;
+    };
+  }
+  int a2;
+};

base-commit: d08677c11dc4b43cc8bab862d1c986563897ce3f
-- 
2.29.2



More information about the Gcc-patches mailing list