diff mbox

[/,RFC] PR 39681

Message ID 4FAFE54C.2080104@oracle.com
State New
Headers show

Commit Message

Paolo Carlini May 13, 2012, 4:46 p.m. UTC 
Hi,

I had a look to this rather old PR, which doesn't seem particularly 
high-priority, to be honest. It's about the error message we produce for 
things like:

int main()
{
int* p = new foo;
}

where, besides the obvious error message about foo, we also print:

39681.C:3:16: error: expected ‘,’ or ‘;’ before ‘foo’

which seems redundant and boring. The first thing I noticed is that in 
cp_parser_new_type_id, at variance with cp_parser_type_id, when 
cp_parser_type_specifier_seq sets error_mark_node as 
type_specifier_seq.type we don't notice. If change it to also return 
error_mark_node we have a chance to do something in the immediate 
callers. That would allow to avoid the above error message. However, I 
noticed that I could do the check a couple of levels above, that is in 
cp_parser_init_declarator and cp_parser_late_parse_one_default_arg, and 
this allows to catch more redundant error messages about expected 
semicolons (see the ext and c-c++-common testcases below, I like that *a 
lot* ;): it's just matter of skipping to the end of the statement. I'm 
not 100% sure about many details of this, eg, whether the diagnostics 
could somehow regress in some particular situations. But the patch boots 
and tests fine (the cp_parser_late_parse_one_default_arg bits take care 
of the C++11 NSDMI case, for example, see error8.C).

Finally, as a last remark, the PR also talks about "expected 
type-specifier before ‘foo’" not being an optimal error message, or at 
least not being clear enough. Well, ICC prints something quite similar 
in this case and related ones (only talks about type instead of 
type-specifier, less technical). All in all I'm not convinced we should 
change cp_parser_type_specifier_seq to print something else, but I think 
it would be just an additional tweak.

Thanks,
Paolo.

/////////////////////////
/cp

	PR c++/39681
	* parser.c (cp_parser_new_type_id): Early return error_mark_node
	if the cp_parser_type_specifier_seq call has type_specifier_seq.type
	error_mark_node; tidy.
	(cp_parser_new_expression): Always initialize nelts to NULL_TREE to
	avoid uninitialized warnings.
	(cp_parser_init_declarator, cp_parser_late_parse_one_default_arg):
	Call cp_parser_skip_to_end_of_statement if cp_parser_initializer
	returns error_mark_node.

/testsuite

	PR c++/39681
	* g++.dg/parse/error48.C: New.
	* g++.dg/cpp0x/error8.C: Likewise.
	* g++.dg/ext/utf-cxx98.C: Adjust dg-error directive.
	* g++.dg/ext/utf-dflt2.C: Likewise.
	* g++.dg/ext/utf-gnuxx98.C: Likewise.
	* g++.dg/ext/utf-dflt.C: Likewise.
	* c-c++-common/raw-string-3.c: Likewise.
	* c-c++-common/raw-string-4.c: Likewise.

Comments

Jason Merrill May 15, 2012, 11:43 p.m. UTC | #1 
OK.

Jason
diff mbox

Patch

Index: testsuite/g++.dg/ext/utf-cxx98.C
===================================================================
--- testsuite/g++.dg/ext/utf-cxx98.C	(revision 187435)
+++ testsuite/g++.dg/ext/utf-cxx98.C	(working copy)
@@ -8,9 +8,7 @@  const static char16_t	c0	= 'a';	/* { dg-error "not
 const static char32_t	c1	= 'a';	/* { dg-error "not name a type" } */
 
 const unsigned short	c2	= u'a';	/* { dg-error "not declared" } */
-	/* { dg-error "expected ',' or ';'" "" { target *-*-* } 10 } */
 const unsigned long	c3	= U'a';	/* { dg-error "not declared" } */
-	/* { dg-error "expected ',' or ';'" "" { target *-*-* } 12 } */
 
 #define u	1 +
 #define U	2 +
Index: testsuite/g++.dg/ext/utf-dflt2.C
===================================================================
--- testsuite/g++.dg/ext/utf-dflt2.C	(revision 187435)
+++ testsuite/g++.dg/ext/utf-dflt2.C	(working copy)
@@ -3,7 +3,6 @@ 
 // { dg-options "-std=c++98" }
 
 const void	*s0 = u8"a";		// { dg-error "was not declared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 5 }
 
 #define u8	"a"
 
Index: testsuite/g++.dg/ext/utf-gnuxx98.C
===================================================================
--- testsuite/g++.dg/ext/utf-gnuxx98.C	(revision 187435)
+++ testsuite/g++.dg/ext/utf-gnuxx98.C	(working copy)
@@ -8,9 +8,7 @@  const static char16_t	c0	= 'a';	/* { dg-error "not
 const static char32_t	c1	= 'a';	/* { dg-error "not name a type" } */
 
 const unsigned short	c2	= u'a';	/* { dg-error "not declared" } */
-	/* { dg-error "expected ',' or ';'" "" { target *-*-* } 10 } */
 const unsigned long	c3	= U'a';	/* { dg-error "not declared" } */
-	/* { dg-error "expected ',' or ';'" "" { target *-*-* } 12 } */
 
 #define u	1 +
 #define U	2 +
Index: testsuite/g++.dg/ext/utf-dflt.C
===================================================================
--- testsuite/g++.dg/ext/utf-dflt.C	(revision 187435)
+++ testsuite/g++.dg/ext/utf-dflt.C	(working copy)
@@ -8,9 +8,7 @@  const static char16_t	c0	= 'a';	/* { dg-error "not
 const static char32_t	c1	= 'a';	/* { dg-error "not name a type" } */
 
 const unsigned short	c2	= u'a';	/* { dg-error "not declared" } */
-	/* { dg-error "expected ',' or ';'" "" { target *-*-* } 10 } */
 const unsigned long	c3	= U'a';	/* { dg-error "not declared" } */
-	/* { dg-error "expected ',' or ';'" "" { target *-*-* } 12 } */
 
 #define u	1 +
 #define U	2 +
Index: testsuite/g++.dg/parse/error48.C
===================================================================
--- testsuite/g++.dg/parse/error48.C	(revision 0)
+++ testsuite/g++.dg/parse/error48.C	(revision 0)
@@ -0,0 +1,6 @@ 
+// PR c++/39681
+
+int main()
+{
+  int* p = new foo; // { dg-error "16:expected type-specifier" }
+}
Index: testsuite/g++.dg/cpp0x/error8.C
===================================================================
--- testsuite/g++.dg/cpp0x/error8.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/error8.C	(revision 0)
@@ -0,0 +1,7 @@ 
+// PR c++/39681
+// { dg-do compile { target c++11 } }
+
+struct A
+{
+  int* p = new foo; // { dg-error "16:expected type-specifier" }
+};
Index: testsuite/c-c++-common/raw-string-3.c
===================================================================
--- testsuite/c-c++-common/raw-string-3.c	(revision 187435)
+++ testsuite/c-c++-common/raw-string-3.c	(working copy)
@@ -5,26 +5,26 @@ 
 // { dg-options "-std=c++98" { target c++ } }
 
 const void	*s0	= R"(a)";	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 7 }
+                // { dg-error "expected ',' or ';'" "" { target c } 7 }
 const void	*s1	= uR"(a)";	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 9 }
+                // { dg-error "expected ',' or ';'" "" { target c } 9 }
 const void	*s2	= UR"(a)";	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 11 }
+                // { dg-error "expected ',' or ';'" "" { target c } 11 }
 const void	*s3	= u8R"(a)";	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 13 }
+                // { dg-error "expected ',' or ';'" "" { target c } 13 }
 const void	*s4	= LR"(a)";	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 15 }
+                // { dg-error "expected ',' or ';'" "" { target c } 15 }
 
-const int	i0	= R'a';		// { dg-error "expected ',' or ';'" }
-		// { dg-error "was not declared" "" { target c++ } 18 }
-const int	i1	= uR'a';	// { dg-error "expected ',' or ';'" }
-		// { dg-error "was not declared" "" { target c++ } 20 }
-const int	i2	= UR'a';	// { dg-error "expected ',' or ';'" }
-		// { dg-error "was not declared" "" { target c++ } 22 }
-const int	i3	= u8R'a';	// { dg-error "expected ',' or ';'" }
-		// { dg-error "was not declared" "" { target c++ } 24 }
-const int	i4	= LR'a';	// { dg-error "expected ',' or ';'" }
-		// { dg-error "was not declared" "" { target c++ } 26 }
+const int	i0	= R'a';		// { dg-error "was not declared" "" { target c++ } }
+                // { dg-error "expected ',' or ';'" "" { target c } 18 }
+const int	i1	= uR'a';	// { dg-error "was not declared" "" { target c++ } }
+                // { dg-error "expected ',' or ';'" "" { target c } 20 }
+const int	i2	= UR'a';	// { dg-error "was not declared" "" { target c++ } }
+                // { dg-error "expected ',' or ';'" "" { target c } 22 }
+const int	i3	= u8R'a';	// { dg-error "was not declared" "" { target c++ } }
+                // { dg-error "expected ',' or ';'" "" { target c } 24 }
+const int	i4	= LR'a';	// { dg-error "was not declared" "" { target c++ } }
+                // { dg-error "expected ',' or ';'" "" { target c } 26 }
 
 #define R	"a"
 #define uR	"b"
Index: testsuite/c-c++-common/raw-string-4.c
===================================================================
--- testsuite/c-c++-common/raw-string-4.c	(revision 187435)
+++ testsuite/c-c++-common/raw-string-4.c	(working copy)
@@ -4,15 +4,15 @@ 
 // { dg-options "-std=c++0x" { target c++ } }
 
 const int	i0	= R'a';	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 6 }
+		// { dg-error "expected ',' or ';'" "" { target c } 6 }
 const int	i1	= uR'a';	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 8 }
+		// { dg-error "expected ',' or ';'" "" { target c } 8 }
 const int	i2	= UR'a';	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 10 }
+		// { dg-error "expected ',' or ';'" "" { target c } 10 }
 const int	i3	= u8R'a';	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 12 }
+		// { dg-error "expected ',' or ';'" "" { target c } 12 }
 const int	i4	= LR'a';	// { dg-error "was not declared|undeclared" }
-		// { dg-error "expected ',' or ';'" "" { target *-*-* } 14 }
+		// { dg-error "expected ',' or ';'" "" { target c } 14 }
 
 #define R	1 +
 #define uR	2 +
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 187435)
+++ cp/parser.c	(working copy)
@@ -6657,7 +6657,7 @@  cp_parser_new_expression (cp_parser* parser)
   VEC(tree,gc) *placement;
   tree type;
   VEC(tree,gc) *initializer;
-  tree nelts;
+  tree nelts = NULL_TREE;
   tree ret;
 
   /* Look for the optional `::' operator.  */
@@ -6710,7 +6710,6 @@  cp_parser_new_expression (cp_parser* parser)
 		  "try removing the parentheses around the type-id");
 	  cp_parser_direct_new_declarator (parser);
 	}
-      nelts = NULL_TREE;
     }
   /* Otherwise, there must be a new-type-id.  */
   else
@@ -6780,7 +6779,6 @@  cp_parser_new_type_id (cp_parser* parser, tree *ne
   cp_declarator *declarator;
   cp_declarator *outer_declarator;
   const char *saved_message;
-  tree type;
 
   /* The type-specifier sequence must not contain type definitions.
      (It cannot contain declarations of new types either, but if they
@@ -6795,6 +6793,10 @@  cp_parser_new_type_id (cp_parser* parser, tree *ne
 				&type_specifier_seq);
   /* Restore the old message.  */
   parser->type_definition_forbidden_message = saved_message;
+
+  if (type_specifier_seq.type == error_mark_node)
+    return error_mark_node;
+
   /* Parse the new-declarator.  */
   new_declarator = cp_parser_new_declarator_opt (parser);
 
@@ -6831,8 +6833,7 @@  cp_parser_new_type_id (cp_parser* parser, tree *ne
 	new_declarator = NULL;
     }
 
-  type = groktypename (&type_specifier_seq, new_declarator, false);
-  return type;
+  return groktypename (&type_specifier_seq, new_declarator, false);
 }
 
 /* Parse an (optional) new-declarator.
@@ -15761,6 +15762,8 @@  cp_parser_init_declarator (cp_parser* parser,
 					       &is_non_constant_init);
 	  if (!member_p && processing_template_decl)
 	    finish_lambda_scope ();
+	  if (initializer == error_mark_node)
+	    cp_parser_skip_to_end_of_statement (parser);
 	}
     }
 
@@ -21777,6 +21780,9 @@  cp_parser_late_parse_one_default_arg (cp_parser *p
 
   finish_lambda_scope ();
 
+  if (parsed_arg == error_mark_node)
+    cp_parser_skip_to_end_of_statement (parser);
+
   if (!processing_template_decl)
     {
       /* In a non-template class, check conversions now.  In a template,