diff mbox

Go compiler patch: Fix check for parenthesized go/defer arg

Message ID mcrsj7jrf8y.fsf@google.com
State New
Headers show

Commit Message

Ian Lance Taylor Dec. 6, 2012, 4:11 a.m. UTC 
My patch of a couple of days ago to prohibit a parenthesized argument to
a go or defer statement was faulty.  I tested whether the expression
starts with a parenthesis, but of course there are valid function calls
that start with a parenthesis but for which the entire expression is not
parenthesized.  This patch fixes that oversight.  Bootstrapped and ran
Go testsuite on x86_64-unknown-linux-gnu.  Committed to mainline.

Ian
diff mbox

Patch

diff -r 1fd473e5d3ac go/parse.cc
--- a/go/parse.cc	Wed Dec 05 17:59:34 2012 -0800
+++ b/go/parse.cc	Wed Dec 05 20:08:02 2012 -0800
@@ -141,7 +141,7 @@ 
   while (true)
     {
       ret->push_back(this->expression(PRECEDENCE_NORMAL, may_be_sink,
-				      may_be_composite_lit, NULL));
+				      may_be_composite_lit, NULL, NULL));
 
       const Token* token = this->peek_token();
       if (!token->is_op(OPERATOR_COMMA))
@@ -394,7 +394,7 @@ 
   else
     {
       if (!token->is_op(OPERATOR_ELLIPSIS))
-	length = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+	length = this->expression(PRECEDENCE_NORMAL, false, true, NULL, NULL);
       else if (may_use_ellipsis)
 	{
 	  // An ellipsis is used in composite literals to represent a
@@ -2137,7 +2137,7 @@ 
       bool is_type_switch = false;
       Expression* expr = this->expression(PRECEDENCE_NORMAL, false,
 					  may_be_composite_lit,
-					  &is_type_switch);
+					  &is_type_switch, NULL);
       if (is_type_switch)
 	{
 	  p_type_switch->found = true;
@@ -2404,8 +2404,11 @@ 
 
 // If MAY_BE_SINK is true, this operand may be "_".
 
+// If IS_PARENTHESIZED is not NULL, *IS_PARENTHESIZED is set to true
+// if the entire expression is in parentheses.
+
 Expression*
-Parse::operand(bool may_be_sink)
+Parse::operand(bool may_be_sink, bool* is_parenthesized)
 {
   const Token* token = this->peek_token();
   Expression* ret;
@@ -2581,11 +2584,14 @@ 
       if (token->is_op(OPERATOR_LPAREN))
 	{
 	  this->advance_token();
-	  ret = this->expression(PRECEDENCE_NORMAL, may_be_sink, true, NULL);
+	  ret = this->expression(PRECEDENCE_NORMAL, may_be_sink, true, NULL,
+				 NULL);
 	  if (!this->peek_token()->is_op(OPERATOR_RPAREN))
 	    error_at(this->location(), "missing %<)%>");
 	  else
 	    this->advance_token();
+	  if (is_parenthesized != NULL)
+	    *is_parenthesized = true;
 	  return ret;
 	}
       else if (token->is_op(OPERATOR_LSQUARE))
@@ -2708,11 +2714,12 @@ 
 	      this->unget_token(Token::make_identifier_token(identifier,
 							     is_exported,
 							     location));
-	      val = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+	      val = this->expression(PRECEDENCE_NORMAL, false, true, NULL,
+				     NULL);
 	    }
 	}
       else if (!token->is_op(OPERATOR_LCURLY))
-	val = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+	val = this->expression(PRECEDENCE_NORMAL, false, true, NULL, NULL);
       else
 	{
 	  // This must be a composite literal inside another composite
@@ -2758,7 +2765,7 @@ 
 	  vals->push_back(val);
 
 	  if (!token->is_op(OPERATOR_LCURLY))
-	    val = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+	    val = this->expression(PRECEDENCE_NORMAL, false, true, NULL, NULL);
 	  else
 	    {
 	      // This must be a composite literal inside another
@@ -2931,19 +2938,25 @@ 
 // If IS_TYPE_SWITCH is not NULL, this will recognize a type switch
 // guard (var := expr.("type") using the literal keyword "type").
 
+// If IS_PARENTHESIZED is not NULL, *IS_PARENTHESIZED is set to true
+// if the entire expression is in parentheses.
+
 Expression*
 Parse::primary_expr(bool may_be_sink, bool may_be_composite_lit,
-		    bool* is_type_switch)
+		    bool* is_type_switch, bool* is_parenthesized)
 {
   Location start_loc = this->location();
-  bool is_parenthesized = this->peek_token()->is_op(OPERATOR_LPAREN);
-
-  Expression* ret = this->operand(may_be_sink);
+  bool operand_is_parenthesized = false;
+  bool whole_is_parenthesized = false;
+
+  Expression* ret = this->operand(may_be_sink, &operand_is_parenthesized);
+
+  whole_is_parenthesized = operand_is_parenthesized;
 
   // An unknown name followed by a curly brace must be a composite
   // literal, and the unknown name must be a type.
   if (may_be_composite_lit
-      && !is_parenthesized
+      && !operand_is_parenthesized
       && ret->unknown_expression() != NULL
       && this->peek_token()->is_op(OPERATOR_LCURLY))
     {
@@ -2959,6 +2972,7 @@ 
     {
       if (this->peek_token()->is_op(OPERATOR_LCURLY))
 	{
+	  whole_is_parenthesized = false;
 	  if (!may_be_composite_lit)
 	    {
 	      Type* t = ret->type();
@@ -2968,17 +2982,18 @@ 
 			 _("parentheses required around this composite literal "
 			   "to avoid parsing ambiguity"));
 	    }
-	  else if (is_parenthesized)
+	  else if (operand_is_parenthesized)
 	    error_at(start_loc,
 		     "cannot parenthesize type in composite literal");
 	  ret = this->composite_lit(ret->type(), 0, ret->location());
 	}
       else if (this->peek_token()->is_op(OPERATOR_LPAREN))
 	{
+	  whole_is_parenthesized = false;
 	  Location loc = this->location();
 	  this->advance_token();
 	  Expression* expr = this->expression(PRECEDENCE_NORMAL, false, true,
-					      NULL);
+					      NULL, NULL);
 	  if (this->peek_token()->is_op(OPERATOR_COMMA))
 	    this->advance_token();
 	  if (this->peek_token()->is_op(OPERATOR_ELLIPSIS))
@@ -3014,19 +3029,29 @@ 
     {
       const Token* token = this->peek_token();
       if (token->is_op(OPERATOR_LPAREN))
-	ret = this->call(this->verify_not_sink(ret));
+	{
+	  whole_is_parenthesized = false;
+	  ret = this->call(this->verify_not_sink(ret));
+	}
       else if (token->is_op(OPERATOR_DOT))
 	{
+	  whole_is_parenthesized = false;
 	  ret = this->selector(this->verify_not_sink(ret), is_type_switch);
 	  if (is_type_switch != NULL && *is_type_switch)
 	    break;
 	}
       else if (token->is_op(OPERATOR_LSQUARE))
-	ret = this->index(this->verify_not_sink(ret));
+	{
+	  whole_is_parenthesized = false;
+	  ret = this->index(this->verify_not_sink(ret));
+	}
       else
 	break;
     }
 
+  if (whole_is_parenthesized && is_parenthesized != NULL)
+    *is_parenthesized = true;
+
   return ret;
 }
 
@@ -3108,7 +3133,7 @@ 
 
   Expression* start;
   if (!this->peek_token()->is_op(OPERATOR_COLON))
-    start = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+    start = this->expression(PRECEDENCE_NORMAL, false, true, NULL, NULL);
   else
     {
       mpz_t zero;
@@ -3124,7 +3149,7 @@ 
       if (this->advance_token()->is_op(OPERATOR_RSQUARE))
 	end = Expression::make_nil(this->location());
       else
-	end = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+	end = this->expression(PRECEDENCE_NORMAL, false, true, NULL, NULL);
     }
   if (!this->peek_token()->is_op(OPERATOR_RSQUARE))
     error_at(this->location(), "missing %<]%>");
@@ -3233,12 +3258,16 @@ 
 // If IS_TYPE_SWITCH is not NULL, this will recognize a type switch
 // guard (var := expr.("type") using the literal keyword "type").
 
+// If IS_PARENTHESIZED is not NULL, *IS_PARENTHESIZED is set to true
+// if the entire expression is in parentheses.
+
 Expression*
 Parse::expression(Precedence precedence, bool may_be_sink,
-		  bool may_be_composite_lit, bool* is_type_switch)
+		  bool may_be_composite_lit, bool* is_type_switch,
+		  bool *is_parenthesized)
 {
   Expression* left = this->unary_expr(may_be_sink, may_be_composite_lit,
-				      is_type_switch);
+				      is_type_switch, is_parenthesized);
 
   while (true)
     {
@@ -3295,6 +3324,9 @@ 
 	  return left;
 	}
 
+      if (is_parenthesized != NULL)
+	*is_parenthesized = false;
+      
       Operator op = token->op();
       Location binop_location = token->location();
 
@@ -3310,7 +3342,7 @@ 
       left = this->verify_not_sink(left);
       Expression* right = this->expression(right_precedence, false,
 					   may_be_composite_lit,
-					   NULL);
+					   NULL, NULL);
       left = Expression::make_binary(op, left, right, binop_location);
     }
 }
@@ -3376,9 +3408,12 @@ 
 // If IS_TYPE_SWITCH is not NULL, this will recognize a type switch
 // guard (var := expr.("type") using the literal keyword "type").
 
+// If IS_PARENTHESIZED is not NULL, *IS_PARENTHESIZED is set to true
+// if the entire expression is in parentheses.
+
 Expression*
 Parse::unary_expr(bool may_be_sink, bool may_be_composite_lit,
-		  bool* is_type_switch)
+		  bool* is_type_switch, bool* is_parenthesized)
 {
   const Token* token = this->peek_token();
 
@@ -3395,7 +3430,7 @@ 
       if (this->advance_token()->is_keyword(KEYWORD_CHAN))
 	{
 	  Expression* expr = this->primary_expr(false, may_be_composite_lit,
-						NULL);
+						NULL, NULL);
 	  if (expr->is_error_expression())
 	    return expr;
 	  else if (!expr->is_type_expression())
@@ -3448,7 +3483,8 @@ 
       Operator op = token->op();
       this->advance_token();
 
-      Expression* expr = this->unary_expr(false, may_be_composite_lit, NULL);
+      Expression* expr = this->unary_expr(false, may_be_composite_lit, NULL,
+					  NULL);
       if (expr->is_error_expression())
 	;
       else if (op == OPERATOR_MULT && expr->is_type_expression())
@@ -3464,7 +3500,7 @@ 
     }
   else
     return this->primary_expr(may_be_sink, may_be_composite_lit,
-			      is_type_switch);
+			      is_type_switch, is_parenthesized);
 }
 
 // This is called for the obscure case of
@@ -3747,7 +3783,8 @@ 
 				     may_be_composite_lit,
 				     (p_type_switch == NULL
 				      ? NULL
-				      : &p_type_switch->found));
+				      : &p_type_switch->found),
+				     NULL);
   if (p_type_switch != NULL && p_type_switch->found)
     {
       p_type_switch->name.clear();
@@ -3857,7 +3894,8 @@ 
   go_assert(this->peek_token()->is_op(OPERATOR_CHANOP));
   Location loc = this->location();
   this->advance_token();
-  Expression* val = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+  Expression* val = this->expression(PRECEDENCE_NORMAL, false, true, NULL,
+				     NULL);
   Statement* s = Statement::make_send_statement(channel, val, loc);
   this->gogo_->add_statement(s);
 }
@@ -4092,11 +4130,12 @@ 
   bool is_go = this->peek_token()->is_keyword(KEYWORD_GO);
   Location stat_location = this->location();
 
-  const Token* token = this->advance_token();
+  this->advance_token();
   Location expr_location = this->location();
-  bool is_parenthesized = token->is_op(OPERATOR_LPAREN);
-
-  Expression* expr = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+
+  bool is_parenthesized = false;
+  Expression* expr = this->expression(PRECEDENCE_NORMAL, false, true, NULL,
+				      &is_parenthesized);
   Call_expression* call_expr = expr->call_expression();
   if (is_parenthesized || call_expr == NULL)
     {
@@ -4198,7 +4237,7 @@ 
 	  cond = Expression::make_error(this->location());
 	}
       if (cond == NULL)
-	cond = this->expression(PRECEDENCE_NORMAL, false, false, NULL);
+	cond = this->expression(PRECEDENCE_NORMAL, false, false, NULL, NULL);
     }
 
   this->gogo_->start_block(this->location());
@@ -4329,7 +4368,7 @@ 
 	  if (switch_val == NULL && !type_switch.found)
 	    {
 	      switch_val = this->expression(PRECEDENCE_NORMAL, false, false,
-					    &type_switch.found);
+					    &type_switch.found, NULL);
 	      if (type_switch.found)
 		{
 		  type_switch.name.clear();
@@ -4351,7 +4390,7 @@ 
 	  error_at(token_loc, "invalid variable name");
 	  this->advance_token();
 	  this->expression(PRECEDENCE_NORMAL, false, false,
-			   &type_switch.found);
+			   &type_switch.found, NULL);
 	  if (this->peek_token()->is_op(OPERATOR_SEMICOLON))
 	    this->advance_token();
 	  if (!this->peek_token()->is_op(OPERATOR_LCURLY))
@@ -4854,7 +4893,7 @@ 
 	  // case rv := <-c:
 	  this->advance_token();
 	  Expression* e = this->expression(PRECEDENCE_NORMAL, false, false,
-					   NULL);
+					   NULL, NULL);
 	  Receive_expression* re = e->receive_expression();
 	  if (re == NULL)
 	    {
@@ -4889,7 +4928,7 @@ 
 		  // case rv, rc := <-c:
 		  this->advance_token();
 		  Expression* e = this->expression(PRECEDENCE_NORMAL, false,
-						   false, NULL);
+						   false, NULL, NULL);
 		  Receive_expression* re = e->receive_expression();
 		  if (re == NULL)
 		    {
@@ -4937,13 +4976,13 @@ 
 
   Expression* e;
   if (saw_comma || !this->peek_token()->is_op(OPERATOR_CHANOP))
-    e = this->expression(PRECEDENCE_NORMAL, true, true, NULL);
+    e = this->expression(PRECEDENCE_NORMAL, true, true, NULL, NULL);
   else
     {
       // case <-c:
       *is_send = false;
       this->advance_token();
-      *channel = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+      *channel = this->expression(PRECEDENCE_NORMAL, false, true, NULL, NULL);
 
       // The next token should be ':'.  If it is '<-', then we have
       // case <-c <- v:
@@ -4963,7 +5002,7 @@ 
 	}
       *is_send = false;
       this->advance_token();
-      *channel = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+      *channel = this->expression(PRECEDENCE_NORMAL, false, true, NULL, NULL);
       if (saw_comma)
 	{
 	  // case v, e = <-c:
@@ -4995,7 +5034,7 @@ 
       *is_send = true;
       *channel = this->verify_not_sink(e);
       this->advance_token();
-      *val = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+      *val = this->expression(PRECEDENCE_NORMAL, false, true, NULL, NULL);
       return true;
     }
 
@@ -5142,7 +5181,7 @@ 
       return;
     }
   else
-    *cond = this->expression(PRECEDENCE_NORMAL, false, true, NULL);
+    *cond = this->expression(PRECEDENCE_NORMAL, false, true, NULL, NULL);
   if (!this->peek_token()->is_op(OPERATOR_SEMICOLON))
     error_at(this->location(), "expected semicolon");
   else
@@ -5176,7 +5215,8 @@ 
     error_at(this->location(), "too many variables for range clause");
 
   this->advance_token();
-  Expression* expr = this->expression(PRECEDENCE_NORMAL, false, false, NULL);
+  Expression* expr = this->expression(PRECEDENCE_NORMAL, false, false, NULL,
+				      NULL);
   p_range_clause->range = expr;
 
   bool any_new = false;
@@ -5223,7 +5263,7 @@ 
 
   this->advance_token();
   p_range_clause->range = this->expression(PRECEDENCE_NORMAL, false, false,
-					   NULL);
+					   NULL, NULL);
 
   p_range_clause->index = vals->front();
   if (vals->size() == 1)
diff -r 1fd473e5d3ac go/parse.h
--- a/go/parse.h	Wed Dec 05 17:59:34 2012 -0800
+++ b/go/parse.h	Wed Dec 05 20:08:02 2012 -0800
@@ -216,7 +216,7 @@ 
 				     Range_clause*, Type_switch*);
   void function_decl(bool saw_nointerface);
   Typed_identifier* receiver();
-  Expression* operand(bool may_be_sink);
+  Expression* operand(bool may_be_sink, bool *is_parenthesized);
   Expression* enclosing_var_reference(Named_object*, Named_object*,
 				      Location);
   Expression* composite_lit(Type*, int depth, Location);
@@ -224,15 +224,16 @@ 
   Expression* create_closure(Named_object* function, Enclosing_vars*,
 			     Location);
   Expression* primary_expr(bool may_be_sink, bool may_be_composite_lit,
-			   bool* is_type_switch);
+			   bool* is_type_switch, bool* is_parenthesized);
   Expression* selector(Expression*, bool* is_type_switch);
   Expression* index(Expression*);
   Expression* call(Expression*);
   Expression* expression(Precedence, bool may_be_sink,
-			 bool may_be_composite_lit, bool* is_type_switch);
+			 bool may_be_composite_lit, bool* is_type_switch,
+			 bool *is_parenthesized);
   bool expression_may_start_here();
   Expression* unary_expr(bool may_be_sink, bool may_be_composite_lit,
-			 bool* is_type_switch);
+			 bool* is_type_switch, bool* is_parenthesized);
   Type* reassociate_chan_direction(Channel_type*, Location);
   Expression* qualified_expr(Expression*, Location);
   Expression* id_to_expression(const std::string&, Location);