From patchwork Mon Feb 5 01:50:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 869127 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-472591-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="eUVjaqmw"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zZVsv1Wd1z9s7F for ; Mon, 5 Feb 2018 12:50:43 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:content-type; q= dns; s=default; b=i4Jv5uvF6R61+FGNBkq7Fgbl4s/UeMZvbebIZQ78nnV7iv IMD/wH7+C2rQIOf8CU9q00wASKJEtu7MfycCOghGhz00GkYLYIjmMahO6QsnJtT7 B76NokuopNsXgg2W+QmkkxI+ZfoLNUIJQlqDpYLSM8bNP+fb6GDprgOiSFnFw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:content-type; s= default; bh=kzeQnzciPRYqA/H9zENd23Egltw=; b=eUVjaqmwnyGLITrFLWhJ UeARz1QqTZzpgJGkR2fz6RNuX+FCpHH/NXMzjgmZHd666sDdYun1z3Hhx6Y3El6Q ut3XzVqQsyMi8jFSrO2NPl6hKzGLZmhbyVRuL/HvWOWs3P7xxLp498351+OqWraX gE2FGA47cQUtyEaMlpHs+FU= Received: (qmail 33791 invoked by alias); 5 Feb 2018 01:50:35 -0000 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 Received: (qmail 33775 invoked by uid 89); 5 Feb 2018 01:50:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=cleans, 54598 X-HELO: mail-wm0-f53.google.com Received: from mail-wm0-f53.google.com (HELO mail-wm0-f53.google.com) (74.125.82.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 05 Feb 2018 01:50:33 +0000 Received: by mail-wm0-f53.google.com with SMTP id v71so22810018wmv.2 for ; Sun, 04 Feb 2018 17:50:33 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=5JE25P3xVNtzpIHmLvtQMXOAIrznC/xs72erRUUUZb0=; b=tKuHPrePGWpT5WWmX4S8Eab3G0dZvi5oF2Z+4nGZsmOUq5WEc+H4ua0xVGBWwi/Plk Jjm1Dd7S7dd7JPFlAW7VSIJy6wqjJg4ktUM+I2DMmUlTZQNT1c4pu4J+u1p+LAdEQQj+ xnkk6/4mMIprm8PFpjpSIidsSDlC+7FH8KozCuGOqbop2LKtfNYoLF7QAwMKBuvktWDQ kFNiMotchc9Ty5YySdgOCUAHEgAJAike3E3NohoZukeiFtZrhmpmHGkiYd0XA+IIP0fu W6xaOdgGDVQ74u2BDrii3pehEtDLb+re3VZLiAxnoiliVzhkv4cIjEK2bTud+JGr0R0B 4aJw== X-Gm-Message-State: AKwxytfrFzjew9H6/c/p8A9rYxvcFTYaMYmxaHwufiWF++D+8n7PdGZT wXvug5LAC2jE4CnaR+GnPdDBu2qqaQ+53ZiUJXXLi0ZM X-Google-Smtp-Source: AH8x226Z502wPzx1mi1Z3ol+OiA3eEbZXTAhn+n/szCa8uQjNhF0y0liRXRKteoBCIT8MA/qzUL3zByVtzahF+I3lHM= X-Received: by 10.80.194.194 with SMTP id u2mr24456438edf.84.1517795431103; Sun, 04 Feb 2018 17:50:31 -0800 (PST) MIME-Version: 1.0 Received: by 10.80.134.56 with HTTP; Sun, 4 Feb 2018 17:50:30 -0800 (PST) From: Ian Lance Taylor Date: Sun, 4 Feb 2018 17:50:30 -0800 Message-ID: Subject: Go patch committed: In range, evaluate array if it has receives or calls To: gcc-patches , gofrontend-dev@googlegroups.com The recent patch to the Go frontend to not evaluate an array value in a range statement with a single iteration variable was too aggressive. The array value should be evaluated if the expression has any calls or receive expressions. This patch fixes that, reusing the existing code that checks whether len/cap of an array is a constant. This patch also cleans up the use of _ as the second variable in a forrange; it was previous inconsistent depending on whether the statement used = or :=. This is a follow-on to https://golang.org/issue/22313. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. Ian Index: gcc/go/gofrontend/MERGE =================================================================== --- gcc/go/gofrontend/MERGE (revision 257376) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -312af623f48633989e9eb6e559ede84a23998ece +5031f878a761bf83f5f96710d62f83e2dc5ecf04 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: gcc/go/gofrontend/expressions.cc =================================================================== --- gcc/go/gofrontend/expressions.cc (revision 257376) +++ gcc/go/gofrontend/expressions.cc (working copy) @@ -7957,8 +7957,10 @@ class Find_call_expression : public Trav int Find_call_expression::expression(Expression** pexpr) { - if ((*pexpr)->call_expression() != NULL - || (*pexpr)->receive_expression() != NULL) + Expression* expr = *pexpr; + if (!expr->is_constant() + && (expr->call_expression() != NULL + || expr->receive_expression() != NULL)) { this->found_ = true; return TRAVERSE_EXIT; @@ -7966,6 +7968,24 @@ Find_call_expression::expression(Express return TRAVERSE_CONTINUE; } +// Return whether calling len or cap on EXPR, of array type, is a +// constant. The language spec says "the expressions len(s) and +// cap(s) are constants if the type of s is an array or pointer to an +// array and the expression s does not contain channel receives or +// (non-constant) function calls." + +bool +Builtin_call_expression::array_len_is_constant(Expression* expr) +{ + go_assert(expr->type()->deref()->array_type() != NULL + && !expr->type()->deref()->is_slice_type()); + if (expr->is_constant()) + return true; + Find_call_expression find_call; + Expression::traverse(&expr, &find_call); + return !find_call.found(); +} + // Return whether this is constant: len of a string constant, or len // or cap of an array, or unsafe.Sizeof, unsafe.Offsetof, // unsafe.Alignof. @@ -7993,19 +8013,9 @@ Builtin_call_expression::do_is_constant( && !arg_type->points_to()->is_slice_type()) arg_type = arg_type->points_to(); - // The len and cap functions are only constant if there are no - // function calls or channel operations in the arguments. - // Otherwise we have to make the call. - if (!arg->is_constant()) - { - Find_call_expression find_call; - Expression::traverse(&arg, &find_call); - if (find_call.found()) - return false; - } - if (arg_type->array_type() != NULL - && arg_type->array_type()->length() != NULL) + && arg_type->array_type()->length() != NULL + && Builtin_call_expression::array_len_is_constant(arg)) return true; if (this->code_ == BUILTIN_LEN && arg_type->is_string_type()) Index: gcc/go/gofrontend/expressions.h =================================================================== --- gcc/go/gofrontend/expressions.h (revision 257357) +++ gcc/go/gofrontend/expressions.h (working copy) @@ -2406,6 +2406,11 @@ class Builtin_call_expression : public C is_builtin() { return true; } + // Return whether EXPR, of array type, is a constant if passed to + // len or cap. + static bool + array_len_is_constant(Expression* expr); + protected: // This overrides Call_expression::do_lower. Expression* Index: gcc/go/gofrontend/parse.cc =================================================================== --- gcc/go/gofrontend/parse.cc (revision 257374) +++ gcc/go/gofrontend/parse.cc (working copy) @@ -5459,8 +5459,7 @@ Parse::range_clause_decl(const Typed_ide no->var_value()->set_type_from_range_value(); if (is_new) any_new = true; - if (!Gogo::is_sink_name(pti->name())) - p_range_clause->value = Expression::make_var_reference(no, location); + p_range_clause->value = Expression::make_var_reference(no, location); } if (!any_new) Index: gcc/go/gofrontend/statements.cc =================================================================== --- gcc/go/gofrontend/statements.cc (revision 257357) +++ gcc/go/gofrontend/statements.cc (working copy) @@ -5311,11 +5311,12 @@ For_range_statement::do_lower(Gogo* gogo // constant, then we do not evaluate the range variable. len(x) is // a contant if x is a string constant or if x is an array. If x is // a constant then evaluating it won't make any difference, so the - // only case to consider is when x is an array. + // only case to consider is when x is an array whose length is constant. bool eval = true; - if (this->value_var_ == NULL + if ((this->value_var_ == NULL || this->value_var_->is_sink_expression()) && range_type->array_type() != NULL - && !range_type->is_slice_type()) + && !range_type->is_slice_type() + && Builtin_call_expression::array_len_is_constant(this->range_)) eval = false; Location loc = this->location(); @@ -5341,7 +5342,7 @@ For_range_statement::do_lower(Gogo* gogo temp_block->add_statement(index_temp); Temporary_statement* value_temp = NULL; - if (this->value_var_ != NULL) + if (this->value_var_ != NULL && !this->value_var_->is_sink_expression()) { value_temp = Statement::make_temporary(value_type, NULL, loc); temp_block->add_statement(value_temp); @@ -5393,7 +5394,7 @@ For_range_statement::do_lower(Gogo* gogo Statement* assign; Expression* index_ref = Expression::make_temporary_reference(index_temp, loc); - if (this->value_var_ == NULL) + if (this->value_var_ == NULL || this->value_var_->is_sink_expression()) assign = Statement::make_assignment(this->index_var_, index_ref, loc); else {