Patchwork [C++] PR 34892

login
register
mail settings
Submitter Paolo Carlini
Date Oct. 25, 2012, 12:44 a.m.
Message ID <50888B56.30701@oracle.com>
Download mbox | patch
Permalink /patch/194013/
State New
Headers show

Comments

Paolo Carlini - Oct. 25, 2012, 12:44 a.m.
Hi again,

On 10/24/2012 09:53 PM, Jason Merrill wrote:
> The parm shouldn't be a parameter pack in that case; the ... is part 
> of the default argument, not the parameter.
I tracked down where this goes wrong, in the parser: the below 
recognizes when cp_parser_parameter_declaration doesn't parse a valid 
default argument and in that case "*is_parameter_pack = true;" doesn't 
happen. This means we have a very good diagnostics, only a single error 
message and nothing else, not even the maybe_warn_variadic_templates 
message in C++98 mode because I don't think the code can make any sense 
in any C++ dialect whatsoever (the latter would be easy to change of course)

Tested x86_64-linux, as usual.

Thanks,
Paolo.

PS: interestingly, detecting error_mark_node toward the end of 
cp_parser_parameter_declaration and turning it into a NULL_TREE leads to 
a single regression, at the end of pr39060.C, because NULL_TREE implies 
that we don't do anymore the diagnostics about default argument part of 
duplicate_decls (which ICC also gives). Besides that, for 34892 we give 
like 6 different error messages ;)

////////////////////////
/cp
2012-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/34892
	* parser.c (cp_parser_template_parameter): Notice when parsing the
	default argument goes wrong in cp_parser_parameter_declaration and
	don't set is_parameter_pack in that case.

/testsuite
2012-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/34892
	* g++.dg/template/crash114.C: New.
Jason Merrill - Oct. 25, 2012, 1:53 a.m.
We can stop it even sooner:

>   /* If the next token is an ellipsis, and we don't already have it
>      marked as a parameter pack, then we have a parameter pack (that
>      has no declarator).  */
>   if (!*is_parameter_pack
>       && cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS)
>       && declarator_can_be_parameter_pack (parameter_declarator->declarator))

We shouldn't even consider this if there was a default argument.

Jason

Patch

Index: testsuite/g++.dg/template/crash114.C
===================================================================
--- testsuite/g++.dg/template/crash114.C	(revision 0)
+++ testsuite/g++.dg/template/crash114.C	(working copy)
@@ -0,0 +1,5 @@ 
+// PR c++/34892
+
+template<int=..., int=0> struct A {};  // { dg-error "expected primary" }
+
+A<0> a;
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 192786)
+++ cp/parser.c	(working copy)
@@ -12267,9 +12267,18 @@  cp_parser_template_parameter (cp_parser* parser, b
     {
       /* Consume the `...'.  */
       cp_lexer_consume_token (parser->lexer);
-      maybe_warn_variadic_templates ();
-      
-      *is_parameter_pack = true;
+
+      /* For an erroneous input like (c++/34892):
+
+	   template<int=..., int=0> struct A {};
+
+	 we gave an error in cp_parser_parameter_declaration and returned a
+	 parameter_declarator with error_mark_node as default_argument.  */
+      if (parameter_declarator->default_argument != error_mark_node)
+	{
+	  maybe_warn_variadic_templates ();
+	  *is_parameter_pack = true;
+	}
     }
   /* We might end up with a pack expansion as the type of the non-type
      template parameter, in which case this is a non-type template