[C++ PATCH] decls with error_node type

[C++ PATCH] decls with error_node type

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

I discovered we could occasionally create TYPE_DECLs with 
error_mark_node type, and then successfully push them into the symbol table!

I don't think we should be doing this -- it means TREE_TYPE of a 
TYPE_DECL might not be a type, with resulting ICE fallout.  (When we 
cons up a VAR_DECL to cope with an undefined lookup we give it int type, 
to avoid this kind of thing.)

This patch stops grokdeclarator doing this, by immediately returning 
error_mark_node.  And added an assert to push_template_decl_real, where 
I noticed the problem occurring.

The fallout is some error cascade on symbol lookups that now fail. 
error33.C was particularly screwy:

   typedef void (A::T) ();

would pushing a T into the current scope.

Jason, ok?

nathan
-- 
Nathan Sidwell

[-- Attachment #2: terror.diff --]
[-- Type: text/x-patch, Size: 3494 bytes --]

2019-05-31  Nathan Sidwell  <nathan@acm.org>

	* decl.c (grokdeclarator): Don't create decls with error_mark_node
	type.
	* pt.c (push_template_decl_real): Assert decl's type not
	error_mark_node.

	* g++.dg/concepts/pr84423-1.C: Add more expected errors.
	* g++.dg/cpp0x/auto39.C: Likewise.
	* g++.dg/parse/error32.C: Likewise.
	* g++.dg/parse/error33.C: Likewise.

Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 271811)
+++ gcc/cp/decl.c	(working copy)
@@ -12082,11 +12082,9 @@ grokdeclarator (const cp_declarator *dec
       if (type_uses_auto (type))
 	{
-	  if (alias_p)
-	    error_at (declspecs->locations[ds_type_spec],
-		      "%<auto%> not allowed in alias declaration");
-	  else
-	    error_at (declspecs->locations[ds_type_spec],
-		      "typedef declared %<auto%>");
-	  type = error_mark_node;
+	  error_at (declspecs->locations[ds_type_spec],
+		    alias_p
+		    ? G_("%<auto%> not allowed in alias declaration")
+		    : G_("typedef declared %<auto%>"));
+	  return error_mark_node;
 	}
 
@@ -12097,5 +12095,5 @@ grokdeclarator (const cp_declarator *dec
 	{
 	  error ("typedef name may not be a nested-name-specifier");
-	  type = error_mark_node;
+	  return error_mark_node;
 	}
 
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 271811)
+++ gcc/cp/pt.c	(working copy)
@@ -5450,4 +5450,6 @@ push_template_decl_real (tree decl, bool
     return error_mark_node;
 
+  gcc_checking_assert (TREE_TYPE (decl) != error_mark_node);
+
   /* See if this is a partial specialization.  */
   is_partial = ((DECL_IMPLICIT_TYPEDEF_P (decl)
Index: gcc/testsuite/g++.dg/concepts/pr84423-1.C
===================================================================
--- gcc/testsuite/g++.dg/concepts/pr84423-1.C	(revision 271811)
+++ gcc/testsuite/g++.dg/concepts/pr84423-1.C	(working copy)
@@ -6,3 +6,3 @@ template<typename> using A = auto;  // {
 template<template<typename> class> struct B {};
 
-B<A> b;
+B<A> b;  // { dg-error "not declared|invalid" }
Index: gcc/testsuite/g++.dg/cpp0x/auto39.C
===================================================================
--- gcc/testsuite/g++.dg/cpp0x/auto39.C	(revision 271811)
+++ gcc/testsuite/g++.dg/cpp0x/auto39.C	(working copy)
@@ -4,3 +4,3 @@
 typedef auto T;     // { dg-error "9:typedef declared 'auto'" }
 
-void foo() { T(); }
+void foo() { T(); } // { dg-error "not declared" }
Index: gcc/testsuite/g++.dg/parse/error32.C
===================================================================
--- gcc/testsuite/g++.dg/parse/error32.C	(revision 271811)
+++ gcc/testsuite/g++.dg/parse/error32.C	(working copy)
@@ -8,5 +8,5 @@ typedef void (A::T)(); /* { dg-error "ty
 void foo()
 {
-  T t;
+  T t; /* { dg-error "was not declared" } */
   t; /* { dg-error "was not declared" } */
 }
Index: gcc/testsuite/g++.dg/parse/error33.C
===================================================================
--- gcc/testsuite/g++.dg/parse/error33.C	(revision 271811)
+++ gcc/testsuite/g++.dg/parse/error33.C	(working copy)
@@ -9,8 +9,9 @@ struct A
 typedef void (A::T)(); /* { dg-error "typedef name may not be a nested" } */
 
-void bar(T); /* { dg-message "note: declared here" } */
+void bar(T); /* { dg-error "declared void" } */
+// { dg-error "not declared" "" { target *-*-* } .-1 }
 
 void baz()
 {
-  bar(&A::foo); /* { dg-error "too many arguments" } */
+  bar(&A::foo); /* { dg-error "not declared" } */
 }

Re: [C++ PATCH] decls with error_node type

[Jason Merrill]

On 5/31/19 1:39 PM, Nathan Sidwell wrote:
> I discovered we could occasionally create TYPE_DECLs with 
> error_mark_node type, and then successfully push them into the symbol 
> table!
> 
> I don't think we should be doing this -- it means TREE_TYPE of a 
> TYPE_DECL might not be a type, with resulting ICE fallout.  (When we 
> cons up a VAR_DECL to cope with an undefined lookup we give it int type, 
> to avoid this kind of thing.)
> 
> This patch stops grokdeclarator doing this, by immediately returning 
> error_mark_node.  And added an assert to push_template_decl_real, where 
> I noticed the problem occurring.

> The fallout is some error cascade on symbol lookups that now fail. 

Right, that's why we had the previous behavior, to avoid this extra 
noise.  And then various places check error_operand_p to avoid trying to 
do anything with such a declaration.

Perhaps lookup should return error_mark_node rather than a _DECL with 
error_mark_node type?

Jason

Re: [C++ PATCH] decls with error_node type

[Nathan Sidwell]

On 6/3/19 1:30 PM, Jason Merrill wrote:
> On 5/31/19 1:39 PM, Nathan Sidwell wrote:

>> The fallout is some error cascade on symbol lookups that now fail. 
> 
> Right, that's why we had the previous behavior, to avoid this extra 
> noise.  And then various places check error_operand_p to avoid trying to 
> do anything with such a declaration.
> 
> Perhaps lookup should return error_mark_node rather than a _DECL with 
> error_mark_node type?

Good idea, but still fails in simlar ways.   I think that's because with 
the curren tbehaviour, it can parse suchan erronous typedef name as a 
declspec seq -- and a var-decl as a var (I now see we set their type to 
error_mark_node too).    This is looking like a rat hole :(

nathan

-- 
Nathan Sidwell