[C++] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more (Take 2)

Message ID b7b9dc2a-c5f2-a90c-0240-4520f9473ea3@oracle.com
State New
Headers show
Series
  • [C++] PR 71169 ("[7/8 Regression] ICE on invalid C++ code in pop_nested_class"), PR 71832 and more (Take 2)
Related show

Commit Message

Paolo Carlini March 9, 2018, 3:56 a.m.
Hi again,

eventually I'm back with sort-of a synthesis of the various ideas, thus 
something I believe very close in spirit to my original RFC, but much 
more direct, using CLASSTYPE_TI_ARGS (eh!) and Jason' helper. And I 
managed to see that we want to vec_safe_truncate the unparsed entities - 
better late than never.

Anyway, another round of testing is in progress (in v3), all good so far.

Cheers, Paolo.

////////////////////
/cp
2018-03-09  Jason Merrill  <jason@redhat.com>
	    Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/71169
	PR c++/71832
	* pt.c (any_erroneous_template_args_p): New.
	* cp-tree.h (any_erroneous_template_args_p): Declare it.
	* parser.c (cp_parser_class_specifier_1): Use it.

/testsuite
2018-03-09  Jason Merrill  <jason@redhat.com>
	    Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/71169
	PR c++/71832
	* g++.dg/cpp0x/pr71169.C: New.
	* g++.dg/cpp0x/pr71169-2.C: Likewise.
	* g++.dg/cpp0x/pr71832.C: Likewise.

Comments

Paolo Carlini March 9, 2018, 9:14 a.m. | #1
.. after a few hours of sleep, I realize that in principle we could 
also call any_erroneous_template_args_p directly. In that case however, 
I believe get_template_info does nothing 100% of the case, not sure we 
want to do that.

Paolo.
Jason Merrill March 9, 2018, 3:17 p.m. | #2
You should be able to say

  if (any_erroneous_template_args (type))

and I don't think you need the goto; the normal logic should handle
the truncated vecs fine.

On Fri, Mar 9, 2018 at 4:14 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> .. after a few hours of sleep, I realize that in principle we could also

> call any_erroneous_template_args_p directly. In that case however, I believe

> get_template_info does nothing 100% of the case, not sure we want to do

> that.

>

> Paolo.
Paolo Carlini March 9, 2018, 3:40 p.m. | #3
Hi,

On 09/03/2018 16:17, Jason Merrill wrote:
> You should be able to say

>

>    if (any_erroneous_template_args (type))

Indeed, that's what I tried to express earlier today, after a cappuccino ;)
> and I don't think you need the goto; the normal logic should handle

> the truncated vecs fine.

It should, otherwise something needs fixing anyway. And since by that 
time we are in error recovery it doesn't make any sense to 
micro-optimize anything. Thus, I'm re-testing (previous iteration was 
Ok) with the below. Ok if it passes?

Thanks,
Paolo.

///////////////////
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 258381)
+++ cp/cp-tree.h	(working copy)
@@ -6558,6 +6558,7 @@ extern int processing_template_parmlist;
 extern bool dependent_type_p			(tree);
 extern bool dependent_scope_p			(tree);
 extern bool any_dependent_template_arguments_p  (const_tree);
+extern bool any_erroneous_template_args_p       (const_tree);
 extern bool dependent_template_p		(tree);
 extern bool dependent_template_id_p		(tree, tree);
 extern bool type_dependent_expression_p		(tree);
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 258381)
+++ cp/parser.c	(working copy)
@@ -22669,6 +22669,16 @@ cp_parser_class_specifier_1 (cp_parser* parser)
       cp_default_arg_entry *e;
       tree save_ccp, save_ccr;
 
+      if (any_erroneous_template_args_p (type))
+	{
+	  /* Skip default arguments, NSDMIs, etc, in order to improve
+	     error recovery (c++/71169, c++/71832).  */
+	  vec_safe_truncate (unparsed_funs_with_default_args, 0);
+	  vec_safe_truncate (unparsed_nsdmis, 0);
+	  vec_safe_truncate (unparsed_classes, 0);
+	  vec_safe_truncate (unparsed_funs_with_definitions, 0);
+	}
+
       /* In a first pass, parse default arguments to the functions.
 	 Then, in a second pass, parse the bodies of the functions.
 	 This two-phased approach handles cases like:
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 258381)
+++ cp/pt.c	(working copy)
@@ -25048,6 +25048,38 @@ any_dependent_template_arguments_p (const_tree arg
   return false;
 }
 
+/* Returns true if ARGS contains any errors.  */
+
+bool
+any_erroneous_template_args_p (const_tree args)
+{
+  int i;
+  int j;
+
+  if (args && TREE_CODE (args) != TREE_VEC)
+    {
+      if (tree ti = get_template_info (args))
+	args = TI_ARGS (ti);
+      else
+	args = NULL_TREE;
+    }
+
+  if (!args)
+    return false;
+  if (args == error_mark_node)
+    return true;
+
+  for (i = 0; i < TMPL_ARGS_DEPTH (args); ++i)
+    {
+      const_tree level = TMPL_ARGS_LEVEL (args, i + 1);
+      for (j = 0; j < TREE_VEC_LENGTH (level); ++j)
+	if (error_operand_p (TREE_VEC_ELT (level, j)))
+	  return true;
+    }
+
+  return false;
+}
+
 /* Returns TRUE if the template TMPL is type-dependent.  */
 
 bool
Index: testsuite/g++.dg/cpp0x/pr71169-2.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169-2.C	(working copy)
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
+
+template<typename>
+struct B
+{
+  int f(int = 0) { return 0; }
+};
+
+int main()
+{
+  B<int> b;
+  return b.f();
+}
Index: testsuite/g++.dg/cpp0x/pr71169.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
Index: testsuite/g++.dg/cpp0x/pr71832.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71832.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71832.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template < typename decltype (0) > struct A  // { dg-error "expected|two or more" }
+{ 
+  void foo () { baz (); }
+  template < typename ... S > void baz () {}
+};
Jason Merrill March 9, 2018, 4:11 p.m. | #4
Hmm, actually I think any_erroneous_template_args will return false
for error_mark_node.  Just moving the error_mark_node check up should
correct that.  OK with that change.

On Fri, Mar 9, 2018 at 10:40 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,

>

> On 09/03/2018 16:17, Jason Merrill wrote:

>>

>> You should be able to say

>>

>>    if (any_erroneous_template_args (type))

>

> Indeed, that's what I tried to express earlier today, after a cappuccino ;)

>>

>> and I don't think you need the goto; the normal logic should handle

>> the truncated vecs fine.

>

> It should, otherwise something needs fixing anyway. And since by that time

> we are in error recovery it doesn't make any sense to micro-optimize

> anything. Thus, I'm re-testing (previous iteration was Ok) with the below.

> Ok if it passes?

>

> Thanks,

> Paolo.

>

> ///////////////////
Paolo Carlini March 9, 2018, 7:51 p.m. | #5
Hi,

On 09/03/2018 17:11, Jason Merrill wrote:
> Hmm, actually I think any_erroneous_template_args will return false

> for error_mark_node.  Just moving the error_mark_node check up should

> correct that.  OK with that change.

Ah, thanks for spotting that: it wasn't clear to me that TI_ARGS cannot 
be error_mark_node neither that skipping the bodies when the type itself 
is error_mark_node cannot hurt. I'm finishing testing the below and will 
apply it if everything goes well.

Thanks!
Paolo.

/////////////////////////
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 258391)
+++ cp/cp-tree.h	(working copy)
@@ -6558,6 +6558,7 @@ extern int processing_template_parmlist;
 extern bool dependent_type_p			(tree);
 extern bool dependent_scope_p			(tree);
 extern bool any_dependent_template_arguments_p  (const_tree);
+extern bool any_erroneous_template_args_p       (const_tree);
 extern bool dependent_template_p		(tree);
 extern bool dependent_template_id_p		(tree, tree);
 extern bool type_dependent_expression_p		(tree);
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 258391)
+++ cp/parser.c	(working copy)
@@ -22669,6 +22669,16 @@ cp_parser_class_specifier_1 (cp_parser* parser)
       cp_default_arg_entry *e;
       tree save_ccp, save_ccr;
 
+      if (any_erroneous_template_args_p (type))
+	{
+	  /* Skip default arguments, NSDMIs, etc, in order to improve
+	     error recovery (c++/71169, c++/71832).  */
+	  vec_safe_truncate (unparsed_funs_with_default_args, 0);
+	  vec_safe_truncate (unparsed_nsdmis, 0);
+	  vec_safe_truncate (unparsed_classes, 0);
+	  vec_safe_truncate (unparsed_funs_with_definitions, 0);
+	}
+
       /* In a first pass, parse default arguments to the functions.
 	 Then, in a second pass, parse the bodies of the functions.
 	 This two-phased approach handles cases like:
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 258391)
+++ cp/pt.c	(working copy)
@@ -25048,6 +25048,39 @@ any_dependent_template_arguments_p (const_tree arg
   return false;
 }
 
+/* Returns true if ARGS contains any errors.  */
+
+bool
+any_erroneous_template_args_p (const_tree args)
+{
+  int i;
+  int j;
+
+  if (args == error_mark_node)
+    return true;
+
+  if (args && TREE_CODE (args) != TREE_VEC)
+    {
+      if (tree ti = get_template_info (args))
+	args = TI_ARGS (ti);
+      else
+	args = NULL_TREE;
+    }
+
+  if (!args)
+    return false;
+
+  for (i = 0; i < TMPL_ARGS_DEPTH (args); ++i)
+    {
+      const_tree level = TMPL_ARGS_LEVEL (args, i + 1);
+      for (j = 0; j < TREE_VEC_LENGTH (level); ++j)
+	if (error_operand_p (TREE_VEC_ELT (level, j)))
+	  return true;
+    }
+
+  return false;
+}
+
 /* Returns TRUE if the template TMPL is type-dependent.  */
 
 bool
Index: testsuite/g++.dg/cpp0x/pr71169-2.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169-2.C	(working copy)
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
+
+template<typename>
+struct B
+{
+  int f(int = 0) { return 0; }
+};
+
+int main()
+{
+  B<int> b;
+  return b.f();
+}
Index: testsuite/g++.dg/cpp0x/pr71169.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
Index: testsuite/g++.dg/cpp0x/pr71832.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71832.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71832.C	(working copy)
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+template < typename decltype (0) > struct A  // { dg-error "expected|two or more" }
+{ 
+  void foo () { baz (); }
+  template < typename ... S > void baz () {}
+};

Patch

Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 258378)
+++ cp/cp-tree.h	(working copy)
@@ -6558,6 +6558,7 @@  extern int processing_template_parmlist;
 extern bool dependent_type_p			(tree);
 extern bool dependent_scope_p			(tree);
 extern bool any_dependent_template_arguments_p  (const_tree);
+extern bool any_erroneous_template_args_p       (const_tree);
 extern bool dependent_template_p		(tree);
 extern bool dependent_template_id_p		(tree, tree);
 extern bool type_dependent_expression_p		(tree);
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 258378)
+++ cp/parser.c	(working copy)
@@ -22669,6 +22669,19 @@  cp_parser_class_specifier_1 (cp_parser* parser)
       cp_default_arg_entry *e;
       tree save_ccp, save_ccr;
 
+      if (type != error_mark_node
+	  && CLASSTYPE_TEMPLATE_INFO (type)
+	  && any_erroneous_template_args_p (CLASSTYPE_TI_ARGS (type)))
+	{
+	  /* Skip default arguments, NSDMIs, etc, in order to improve
+	     error recovery (c++/71169, c++/71832).  */
+	  vec_safe_truncate (unparsed_funs_with_default_args, 0);
+	  vec_safe_truncate (unparsed_nsdmis, 0);
+	  vec_safe_truncate (unparsed_classes, 0);
+	  vec_safe_truncate (unparsed_funs_with_definitions, 0);
+	  goto out;
+	}
+
       /* In a first pass, parse default arguments to the functions.
 	 Then, in a second pass, parse the bodies of the functions.
 	 This two-phased approach handles cases like:
@@ -22745,6 +22758,7 @@  cp_parser_class_specifier_1 (cp_parser* parser)
   else
     vec_safe_push (unparsed_classes, type);
 
+ out:
   /* Put back any saved access checks.  */
   pop_deferring_access_checks ();
 
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 258378)
+++ cp/pt.c	(working copy)
@@ -25048,6 +25048,38 @@  any_dependent_template_arguments_p (const_tree arg
   return false;
 }
 
+/* Returns true if ARGS contains any errors.  */
+
+bool
+any_erroneous_template_args_p (const_tree args)
+{
+  int i;
+  int j;
+
+  if (args && TREE_CODE (args) != TREE_VEC)
+    {
+      if (tree ti = get_template_info (args))
+	args = TI_ARGS (ti);
+      else
+	args = NULL_TREE;
+    }
+
+  if (!args)
+    return false;
+  if (args == error_mark_node)
+    return true;
+
+  for (i = 0; i < TMPL_ARGS_DEPTH (args); ++i)
+    {
+      const_tree level = TMPL_ARGS_LEVEL (args, i + 1);
+      for (j = 0; j < TREE_VEC_LENGTH (level); ++j)
+	if (error_operand_p (TREE_VEC_ELT (level, j)))
+	  return true;
+    }
+
+  return false;
+}
+
 /* Returns TRUE if the template TMPL is type-dependent.  */
 
 bool
Index: testsuite/g++.dg/cpp0x/pr71169-2.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169-2.C	(working copy)
@@ -0,0 +1,19 @@ 
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
+
+template<typename>
+struct B
+{
+  int f(int = 0) { return 0; }
+};
+
+int main()
+{
+  B<int> b;
+  return b.f();
+}
Index: testsuite/g++.dg/cpp0x/pr71169.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71169.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71169.C	(working copy)
@@ -0,0 +1,7 @@ 
+// { dg-do compile { target c++11 } }
+
+template <Preconditioner> class A {  // { dg-error "declared" }
+  template <class = int> void m_fn1() {
+    m_fn1();
+    }
+};
Index: testsuite/g++.dg/cpp0x/pr71832.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71832.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr71832.C	(working copy)
@@ -0,0 +1,7 @@ 
+// { dg-do compile { target c++11 } }
+
+template < typename decltype (0) > struct A  // { dg-error "expected|two or more" }
+{ 
+  void foo () { baz (); }
+  template < typename ... S > void baz () {}
+};