From patchwork Mon Mar 28 22:49:12 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 88697 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id ED0941007D1 for ; Tue, 29 Mar 2011 09:49:28 +1100 (EST) Received: (qmail 6875 invoked by alias); 28 Mar 2011 22:49:26 -0000 Received: (qmail 6865 invoked by uid 22791); 28 Mar 2011 22:49:24 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, TW_CC, TW_TM, T_RP_MATCHES_RCVD, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.67) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 28 Mar 2011 22:49:19 +0000 Received: from wpaz21.hot.corp.google.com (wpaz21.hot.corp.google.com [172.24.198.85]) by smtp-out.google.com with ESMTP id p2SMnHxQ020136 for ; Mon, 28 Mar 2011 15:49:18 -0700 Received: from iyb14 (iyb14.prod.google.com [10.241.49.78]) by wpaz21.hot.corp.google.com with ESMTP id p2SMnGXA006250 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Mon, 28 Mar 2011 15:49:16 -0700 Received: by iyb14 with SMTP id 14so3617474iyb.19 for ; Mon, 28 Mar 2011 15:49:16 -0700 (PDT) Received: by 10.42.133.4 with SMTP id f4mr2210598ict.167.1301352556215; Mon, 28 Mar 2011 15:49:16 -0700 (PDT) Received: from coign.google.com ([216.239.45.130]) by mx.google.com with ESMTPS id 19sm3212640ibx.52.2011.03.28.15.49.14 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 28 Mar 2011 15:49:15 -0700 (PDT) From: Ian Lance Taylor To: gcc-patches@gcc.gnu.org, gofrontend-dev@googlegroups.com Subject: Go patch committed: Better error message for send as value Date: Mon, 28 Mar 2011 15:49:12 -0700 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org This patch to gccgo makes it emit better a error message when using the now obsolete syntax of a send expression as a value. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian diff -r 60aecfe6ad83 go/parse.cc --- a/go/parse.cc Mon Mar 28 14:49:45 2011 -0700 +++ b/go/parse.cc Mon Mar 28 15:39:25 2011 -0700 @@ -3153,7 +3153,7 @@ case KEYWORD_MAP: case KEYWORD_STRUCT: case KEYWORD_INTERFACE: - this->simple_stat(true, false, NULL, NULL); + this->simple_stat(true, NULL, NULL, NULL); break; case KEYWORD_GO: case KEYWORD_DEFER: @@ -3206,7 +3206,7 @@ this->unget_token(Token::make_identifier_token(identifier, is_exported, location)); - this->simple_stat(true, false, NULL, NULL); + this->simple_stat(true, NULL, NULL, NULL); } } break; @@ -3221,14 +3221,14 @@ location); } else if (!token->is_op(OPERATOR_SEMICOLON)) - this->simple_stat(true, false, NULL, NULL); + this->simple_stat(true, NULL, NULL, NULL); break; case Token::TOKEN_STRING: case Token::TOKEN_INTEGER: case Token::TOKEN_FLOAT: case Token::TOKEN_IMAGINARY: - this->simple_stat(true, false, NULL, NULL); + this->simple_stat(true, NULL, NULL, NULL); break; default: @@ -3330,10 +3330,12 @@ // EmptyStmt was handled in Parse::statement. // In order to make this work for if and switch statements, if -// RETURN_EXP is true, and we see an ExpressionStat, we return the +// RETURN_EXP is not NULL, and we see an ExpressionStat, we return the // expression rather than adding an expression statement to the // current block. If we see something other than an ExpressionStat, -// we add the statement and return NULL. +// we add the statement, set *RETURN_EXP to true if we saw a send +// statement, and return NULL. The handling of send statements is for +// better error messages. // If P_RANGE_CLAUSE is not NULL, then this will recognize a // RangeClause. @@ -3342,7 +3344,7 @@ // guard (var := expr.("type") using the literal keyword "type"). Expression* -Parse::simple_stat(bool may_be_composite_lit, bool return_exp, +Parse::simple_stat(bool may_be_composite_lit, bool* return_exp, Range_clause* p_range_clause, Type_switch* p_type_switch) { const Token* token = this->peek_token(); @@ -3385,7 +3387,11 @@ } token = this->peek_token(); if (token->is_op(OPERATOR_CHANOP)) - this->send_stmt(this->verify_not_sink(exp)); + { + this->send_stmt(this->verify_not_sink(exp)); + if (return_exp != NULL) + *return_exp = true; + } else if (token->is_op(OPERATOR_PLUSPLUS) || token->is_op(OPERATOR_MINUSMINUS)) this->inc_dec_stat(this->verify_not_sink(exp)); @@ -3404,7 +3410,7 @@ || token->is_op(OPERATOR_ANDEQ) || token->is_op(OPERATOR_BITCLEAREQ)) this->assignment(this->verify_not_sink(exp), p_range_clause); - else if (return_exp) + else if (return_exp != NULL) return this->verify_not_sink(exp); else this->expression_stat(this->verify_not_sink(exp)); @@ -3743,9 +3749,14 @@ this->gogo_->start_block(location); + bool saw_simple_stat = false; Expression* cond = NULL; + bool saw_send_stmt; if (this->simple_stat_may_start_here()) - cond = this->simple_stat(false, true, NULL, NULL); + { + cond = this->simple_stat(false, &saw_send_stmt, NULL, NULL); + saw_simple_stat = true; + } if (cond != NULL && this->peek_token()->is_op(OPERATOR_SEMICOLON)) { // The SimpleStat is an expression statement. @@ -3756,7 +3767,20 @@ { if (this->peek_token()->is_op(OPERATOR_SEMICOLON)) this->advance_token(); - cond = this->expression(PRECEDENCE_NORMAL, false, false, NULL); + else if (saw_simple_stat) + { + if (saw_send_stmt) + error_at(this->location(), + ("send statement used as value; " + "use select for non-blocking send")); + else + error_at(this->location(), + "expected %<;%> after statement in if expression"); + if (!this->expression_may_start_here()) + cond = Expression::make_error(this->location()); + } + if (cond == NULL) + cond = this->expression(PRECEDENCE_NORMAL, false, false, NULL); } this->gogo_->start_block(this->location()); @@ -3809,10 +3833,16 @@ this->gogo_->start_block(location); + bool saw_simple_stat = false; Expression* switch_val = NULL; + bool saw_send_stmt; Type_switch type_switch; if (this->simple_stat_may_start_here()) - switch_val = this->simple_stat(false, true, NULL, &type_switch); + { + switch_val = this->simple_stat(false, &saw_send_stmt, NULL, + &type_switch); + saw_simple_stat = true; + } if (switch_val != NULL && this->peek_token()->is_op(OPERATOR_SEMICOLON)) { // The SimpleStat is an expression statement. @@ -3823,6 +3853,16 @@ { if (this->peek_token()->is_op(OPERATOR_SEMICOLON)) this->advance_token(); + else if (saw_simple_stat) + { + if (saw_send_stmt) + error_at(this->location(), + ("send statement used as value; " + "use select for non-blocking send")); + else + error_at(this->location(), + "expected %<;%> after statement in switch expression"); + } if (!this->peek_token()->is_op(OPERATOR_LCURLY)) { if (this->peek_token()->is_identifier()) @@ -3840,7 +3880,7 @@ if (is_coloneq) { // This must be a TypeSwitchGuard. - switch_val = this->simple_stat(false, true, NULL, + switch_val = this->simple_stat(false, &saw_send_stmt, NULL, &type_switch); if (!type_switch.found) { @@ -4500,11 +4540,19 @@ { // We might be looking at a Condition, an InitStat, or a // RangeClause. - cond = this->simple_stat(false, true, &range_clause, NULL); + bool saw_send_stmt; + cond = this->simple_stat(false, &saw_send_stmt, &range_clause, NULL); if (!this->peek_token()->is_op(OPERATOR_SEMICOLON)) { if (cond == NULL && !range_clause.found) - error_at(this->location(), "parse error in for statement"); + { + if (saw_send_stmt) + error_at(this->location(), + ("send statement used as value; " + "use select for non-blocking send")); + else + error_at(this->location(), "parse error in for statement"); + } } else { @@ -4608,7 +4656,7 @@ else { this->gogo_->start_block(this->location()); - this->simple_stat(false, false, NULL, NULL); + this->simple_stat(false, NULL, NULL, NULL); *post = this->gogo_->finish_block(this->location()); } } @@ -4992,8 +5040,13 @@ token = this->advance_token(); else if (!token->is_eof() || !saw_errors()) { - error_at(this->location(), - "expected %<;%> or newline after top level declaration"); + if (token->is_op(OPERATOR_CHANOP)) + error_at(this->location(), + ("send statement used as value; " + "use select for non-blocking send")); + else + error_at(this->location(), + "expected %<;%> or newline after top level declaration"); this->skip_past_error(OPERATOR_INVALID); } } diff -r 60aecfe6ad83 go/parse.h --- a/go/parse.h Mon Mar 28 14:49:45 2011 -0700 +++ b/go/parse.h Mon Mar 28 15:39:25 2011 -0700 @@ -223,7 +223,7 @@ void statement(Label*); bool statement_may_start_here(); void labeled_stmt(const std::string&, source_location); - Expression* simple_stat(bool, bool, Range_clause*, Type_switch*); + Expression* simple_stat(bool, bool*, Range_clause*, Type_switch*); bool simple_stat_may_start_here(); void statement_list(); bool statement_list_may_start_here();