This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [c++ PATCH] PR 29087 (ice on valid code)


On 9/18/06, Mark Mitchell <mark@codesourcery.com> wrote:
Steven Bosscher wrote:

>       * parser.c (cp_parser_labeled_statement): Return nothing.
>       Rename to cp_parser_label_for_labeled_statement.  Parse only
>       the label, not the statement.
>       (cp_parser_statement): Parse the statement of a labeled-statement
>       from here, using tail recursion.

:REVIEWMAIL: OK

Thanks.



Minor change:

> -/* Parse a labeled-statement.
> +   This function is only called by cp_parser_statement.  We can use
> +   this to avoid extremely deep recursion (e.g. PR29087): In this
> +   function we only parse up to `statement', and then we return to
> +   cp_parser_statement to parse `statement' there.  So the bits we
> +   parse here are:

Please adjust this comment to retain the description of IN_COMPOUND,
which it looks like you removed.  Also, please remove the PR number, as
the comments in the code ought to stand on their own.

Not sure what you mean to say here. Retain the comment about a function argument that I removed? I now mention this in the ChangeLog explicitly.

Just say something like:

/* Parse the label for a labeled-statement, i.e.:

> +   identifier :
> +   case constant-expression :
> +   case constant-expression ... constant-expression :
> +   default :
> +
> +   When a label is parsed without errors, the label is added to the
> +   parse tree by the finish_* functions, so this function doesn't
> +   have to return the label.  */

Done like that.


The rationale is evident from the manual "restart" in the caller.

Okeydokey.


New patch is attached.  I'll assume you agree with the comments in
this patch if I don't hear back from you.

Thanks,

Gr.
Steven
	PR c++/29087
	* parser.c (cp_parser_labeled_statement): Return nothing.  Do
	not take in_statement_expr and in_compound as arguments.  Rename
	to cp_parser_label_for_labeled_statement.  Parse only the label,
	not the statement.
	(cp_parser_statement): Parse the statement of a labeled-statement
	from here, using tail recursion.

Index: parser.c
===================================================================
--- parser.c	(revision 117004)
+++ parser.c	(working copy)
@@ -1451,8 +1451,8 @@ static tree cp_parser_builtin_offsetof
 
 static void cp_parser_statement
   (cp_parser *, tree, bool);
-static tree cp_parser_labeled_statement
-  (cp_parser *, tree, bool);
+static void cp_parser_label_for_labeled_statement
+  (cp_parser *);
 static tree cp_parser_expression_statement
   (cp_parser *, tree);
 static tree cp_parser_compound_statement
@@ -6117,9 +6117,11 @@ cp_parser_statement (cp_parser* parser, 
 	{
 	case RID_CASE:
 	case RID_DEFAULT:
-	  statement = cp_parser_labeled_statement (parser, in_statement_expr,
-						   in_compound);
-	  break;
+	  /* Looks like a labeled-statement with a case label.
+	     Parse the label, and then use tail recursion to parse
+	     the statement.  */
+	  cp_parser_label_for_labeled_statement (parser);
+	  goto restart;
 
 	case RID_IF:
 	case RID_SWITCH:
@@ -6164,8 +6166,13 @@ cp_parser_statement (cp_parser* parser, 
 	 labeled-statement.  */
       token = cp_lexer_peek_nth_token (parser->lexer, 2);
       if (token->type == CPP_COLON)
-	statement = cp_parser_labeled_statement (parser, in_statement_expr,
-						 in_compound);
+	{
+	  /* Looks like a labeled-statement with an ordinary label.
+	     Parse the label, and then use tail recursion to parse
+	     the statement.  */
+	  cp_parser_label_for_labeled_statement (parser);
+	  goto restart;
+	}
     }
   /* Anything that starts with a `{' must be a compound-statement.  */
   else if (token->type == CPP_OPEN_BRACE)
@@ -6215,30 +6222,23 @@ cp_parser_statement (cp_parser* parser, 
     SET_EXPR_LOCATION (statement, statement_location);
 }
 
-/* Parse a labeled-statement.
+/* Parse the label for a labeled-statement, i.e.
 
-   labeled-statement:
-     identifier : statement
-     case constant-expression : statement
-     default : statement
+   identifier :
+   case constant-expression :
+   default :
 
    GNU Extension:
+   case constant-expression ... constant-expression : statement
 
-   labeled-statement:
-     case constant-expression ... constant-expression : statement
-
-   Returns the new CASE_LABEL_EXPR, for a `case' or `default' label.
-   For an ordinary label, returns a LABEL_EXPR.
-
-   IN_COMPOUND is as for cp_parser_statement: true when we're nested
-   inside a compound.  */
+   When a label is parsed without errors, the label is added to the
+   parse tree by the finish_* functions, so this function doesn't
+   have to return the label.  */
 
-static tree
-cp_parser_labeled_statement (cp_parser* parser, tree in_statement_expr,
-			     bool in_compound)
+static void
+cp_parser_label_for_labeled_statement (cp_parser* parser)
 {
   cp_token *token;
-  tree statement = error_mark_node;
 
   /* The next token should be an identifier.  */
   token = cp_lexer_peek_token (parser->lexer);
@@ -6246,7 +6246,7 @@ cp_parser_labeled_statement (cp_parser* 
       && token->type != CPP_KEYWORD)
     {
       cp_parser_error (parser, "expected labeled-statement");
-      return error_mark_node;
+      return;
     }
 
   switch (token->keyword)
@@ -6279,7 +6279,7 @@ cp_parser_labeled_statement (cp_parser* 
 	  expr_hi = NULL_TREE;
 
 	if (parser->in_switch_statement_p)
-	  statement = finish_case_label (expr, expr_hi);
+	  finish_case_label (expr, expr_hi);
 	else
 	  error ("case label %qE not within a switch statement", expr);
       }
@@ -6290,24 +6290,19 @@ cp_parser_labeled_statement (cp_parser* 
       cp_lexer_consume_token (parser->lexer);
 
       if (parser->in_switch_statement_p)
-	statement = finish_case_label (NULL_TREE, NULL_TREE);
+	finish_case_label (NULL_TREE, NULL_TREE);
       else
 	error ("case label not within a switch statement");
       break;
 
     default:
       /* Anything else must be an ordinary label.  */
-      statement = finish_label_stmt (cp_parser_identifier (parser));
+      finish_label_stmt (cp_parser_identifier (parser));
       break;
     }
 
   /* Require the `:' token.  */
   cp_parser_require (parser, CPP_COLON, "`:'");
-  /* Parse the labeled statement.  */
-  cp_parser_statement (parser, in_statement_expr, in_compound);
-
-  /* Return the label, in the case of a `case' or `default' label.  */
-  return statement;
 }
 
 /* Parse an expression-statement.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]