diff mbox

[C++,/,RFC] PR 29234

Message ID 5270FCA7.8000508@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Oct. 30, 2013, 12:33 p.m. UTC
Hi,

I'm having a look at this very old parsing issue. We reject things like:

struct S { void operator () (); };

void foo ()
{
   ( S()() );
}

because the parenthesized S()() triggers an hard error from groktypename 
as called at the end of cp_parser_type_id (called by 
cp_parser_cast_expression), about a function returning a function. Of 
course we have to parse it as an unary-expression.

I scratched my head for a while, but then I noticed that in 
cp_parser_cast_expression, a bit later the cp_parser_type_id call at 
issue, we are *already* using a cp_parser_tokens_start_cast_expression 
which helps use figuring out whether the tokens following the outer ')' 
make sense as a cast-expression. Using it a bit earlier to completely 
avoid calling cp_parser_type_id in the cases at issue, appears to work 
well, for a - negligeable in my opinion - additional computational cost.

Then, cp_parser_unary_expression is actually used. As part of that, 
cp_parser_postfix_expression looks for a compound-literal, but not in 
the robust way we are using elsewhere involving a 
cp_parser_skip_to_closing_parenthesis, but just using tentative parsing 
with cp_parser_type_id. Thus we are incurring again in the same issue. 
Parsing the compound-literal in the same way used elesewhere - thus 
looking for the '{' after the ')', solves the problem. In my personal 
opinion, again, the computational cost shouldn't be too high, because if 
we don't find '{' we completely avoid cp_parser_type_id.

Is this the kind of approach we want to pursue for this issue?

Tested x86_64-linux.

Thanks!
Paolo.

/////////////////////////////
diff mbox

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 204202)
+++ cp/parser.c	(working copy)
@@ -5800,31 +5800,45 @@  cp_parser_postfix_expression (cp_parser *parser, b
 	    && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN))
 	  {
 	    tree initializer = NULL_TREE;
-	    bool saved_in_type_id_in_expr_p;
+	    bool compound_literal_p;
 
 	    cp_parser_parse_tentatively (parser);
 	    /* Consume the `('.  */
 	    cp_lexer_consume_token (parser->lexer);
-	    /* Parse the type.  */
-	    saved_in_type_id_in_expr_p = parser->in_type_id_in_expr_p;
-	    parser->in_type_id_in_expr_p = true;
-	    type = cp_parser_type_id (parser);
-	    parser->in_type_id_in_expr_p = saved_in_type_id_in_expr_p;
-	    /* Look for the `)'.  */
-	    cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN);
+
+	    /* Avoid calling cp_parser_type_id pointlessly, see comment
+	       in cp_parser_cast_expression about c++/29234.  */
+	    cp_lexer_save_tokens (parser->lexer);
+
+	    compound_literal_p
+	      = (cp_parser_skip_to_closing_parenthesis (parser, false, false,
+							/*consume_paren=*/true)
+		 && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE));
+
+	    /* Roll back the tokens we skipped.  */
+	    cp_lexer_rollback_tokens (parser->lexer);
+
+	    if (!compound_literal_p)
+	      cp_parser_simulate_error (parser);
+	    else
+	      {
+		/* Parse the type.  */
+		bool saved_in_type_id_in_expr_p = parser->in_type_id_in_expr_p;
+		parser->in_type_id_in_expr_p = true;
+		type = cp_parser_type_id (parser);
+		parser->in_type_id_in_expr_p = saved_in_type_id_in_expr_p;
+		/* Look for the `)'.  */
+		cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN);
+	      }
+
 	    /* If things aren't going well, there's no need to
 	       keep going.  */
 	    if (!cp_parser_error_occurred (parser))
 	      {
-		if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
-		  {
-		    bool non_constant_p;
-		    /* Parse the brace-enclosed initializer list.  */
-		    initializer = cp_parser_braced_list (parser,
-							 &non_constant_p);
-		  }
-		else
-		  cp_parser_simulate_error (parser);
+		bool non_constant_p;
+		/* Parse the brace-enclosed initializer list.  */
+		initializer = cp_parser_braced_list (parser,
+						     &non_constant_p);
 	      }
 	    /* If that worked, we're definitely looking at a
 	       compound-literal expression.  */
@@ -7559,7 +7573,9 @@  cp_parser_cast_expression (cp_parser *parser, bool
     {
       tree type = NULL_TREE;
       tree expr = NULL_TREE;
+      bool skip_to_closing_ok_p;
       bool compound_literal_p;
+      bool cast_expression_p;
       const char *saved_message;
 
       /* There's no way to know yet whether or not this is a cast.
@@ -7587,21 +7603,44 @@  cp_parser_cast_expression (cp_parser *parser, bool
 	 if the token after the `)' is a `{'.  If so, we are not
 	 looking at a cast-expression.
 
+	 Another tricky case is the following (c++/29234):
+
+         struct S { void operator () (); };
+
+         void foo ()
+         {
+           ( S()() );
+         }
+
+	 where the parenthesized S()(), as a type-id would be parsed as a
+	 function returning a function.  Thus see if the tokens after the
+	 outer `)' are starting a cast-expression.
+
 	 Save tokens so that we can put them back.  */
       cp_lexer_save_tokens (parser->lexer);
-      /* Skip tokens until the next token is a closing parenthesis.
-	 If we find the closing `)', and the next token is a `{', then
+
+      /* Skip tokens until the next token is a closing parenthesis.  */
+      skip_to_closing_ok_p
+	= cp_parser_skip_to_closing_parenthesis (parser, false, false,
+						 /*consume_paren=*/true);
+
+      /* If we find the closing `)', and the next token is a `{', then
 	 we are looking at a compound-literal.  */
       compound_literal_p
-	= (cp_parser_skip_to_closing_parenthesis (parser, false, false,
-						  /*consume_paren=*/true)
+	= (skip_to_closing_ok_p
 	   && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE));
+
+      /* Alternately, we may be looking at a cast-expression.  */
+      cast_expression_p
+	= (skip_to_closing_ok_p
+	   && cp_parser_tokens_start_cast_expression (parser));
+
       /* Roll back the tokens we skipped.  */
       cp_lexer_rollback_tokens (parser->lexer);
-      /* If we were looking at a compound-literal, simulate an error
-	 so that the call to cp_parser_parse_definitely below will
-	 fail.  */
-      if (compound_literal_p)
+      /* If we were looking at a compound-literal or we can't possibly
+	 be looking at a cast-expression, simulate an error so that the
+	 call to cp_parser_parse_definitely below will fail.  */
+      if (compound_literal_p || !cast_expression_p)
 	cp_parser_simulate_error (parser);
       else
 	{
@@ -7620,8 +7659,7 @@  cp_parser_cast_expression (cp_parser *parser, bool
       /* At this point this can only be either a cast or a
 	 parenthesized ctor such as `(T ())' that looks like a cast to
 	 function returning T.  */
-      if (!cp_parser_error_occurred (parser)
-	  && cp_parser_tokens_start_cast_expression (parser))
+      if (!cp_parser_error_occurred (parser))
 	{
 	  cp_parser_parse_definitely (parser);
 	  expr = cp_parser_cast_expression (parser,
Index: testsuite/g++.dg/parse/pr29234.C
===================================================================
--- testsuite/g++.dg/parse/pr29234.C	(revision 0)
+++ testsuite/g++.dg/parse/pr29234.C	(working copy)
@@ -0,0 +1,16 @@ 
+// PR c++/29234
+
+struct S { void operator()(); };
+
+void foo ()
+{
+  ( S()() );
+}
+
+struct C { void operator[](C); };
+
+void bar ()
+{
+  C x;
+  ( C()[x] );
+}