From patchwork Tue Jun 22 18:39:38 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 56564 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 27770B6F16 for ; Wed, 23 Jun 2010 04:39:59 +1000 (EST) Received: (qmail 16402 invoked by alias); 22 Jun 2010 18:39:58 -0000 Received: (qmail 16382 invoked by uid 22791); 22 Jun 2010 18:39:56 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, TW_CC, 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) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 22 Jun 2010 18:39:50 +0000 Received: from wpaz24.hot.corp.google.com (wpaz24.hot.corp.google.com [172.24.198.88]) by smtp-out.google.com with ESMTP id o5MIdmDX030463 for ; Tue, 22 Jun 2010 11:39:48 -0700 Received: from pwi1 (pwi1.prod.google.com [10.241.219.1]) by wpaz24.hot.corp.google.com with ESMTP id o5MIdkSh008883 for ; Tue, 22 Jun 2010 11:39:47 -0700 Received: by pwi1 with SMTP id 1so1157560pwi.36 for ; Tue, 22 Jun 2010 11:39:46 -0700 (PDT) Received: by 10.143.153.34 with SMTP id f34mr5424798wfo.2.1277231986725; Tue, 22 Jun 2010 11:39:46 -0700 (PDT) Received: from coign.google.com (dhcp-172-22-126-240.mtv.corp.google.com [172.22.126.240]) by mx.google.com with ESMTPS id e40sm728626wfj.23.2010.06.22.11.39.45 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 22 Jun 2010 11:39:46 -0700 (PDT) To: gcc-patches@gcc.gnu.org Subject: [gccgo] Clean up addressability From: Ian Lance Taylor Date: Tue, 22 Jun 2010 11:39:38 -0700 Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.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 Go frontend patch cleans up addressable expressions. It separate the test for whether an expression is addressable from marking that we are taking the address of the expression. This fixes cases like &a[0:1][0] which are valid but which in the earlier scheme caused an error message. Committed to gccgo branch. Ian diff -r be235d7367a8 go/expressions.cc --- a/go/expressions.cc Tue Jun 22 10:39:31 2010 -0700 +++ b/go/expressions.cc Tue Jun 22 11:29:31 2010 -0700 @@ -135,24 +135,6 @@ this->warn_unused_value(); } -// This virtual function is the default behaviour for taking the -// address of an expression. - -bool -Expression::do_address_taken(source_location location, bool) -{ - this->report_address_taken_error(location); - return false; -} - -// Report an error taking the address of an expression. - -void -Expression::report_address_taken_error(source_location location) -{ - error_at(location, "invalid operand for unary %<&%>"); -} - // This is the default implementation of the function to handle // decrementing the reference count of the old value of an lvalue. // Any expression which may be an l-value must implement this. @@ -813,11 +795,11 @@ { return this; } bool - do_is_value() const - { return true; } - - bool - do_address_taken(source_location, bool) + do_is_lvalue() const + { return true; } + + bool + do_is_addressable() const { return true; } Expression* @@ -930,21 +912,15 @@ // Something takes the address of this variable. This means that we // may want to move the variable onto the heap. -bool -Var_expression::do_address_taken(source_location, bool escapes) +void +Var_expression::do_address_taken(bool escapes) { if (!escapes) - return true; + ; else if (this->variable_->is_variable()) - { - this->variable_->var_value()->set_address_taken(); - return true; - } + this->variable_->var_value()->set_address_taken(); else if (this->variable_->is_result_variable()) - { - this->variable_->result_var_value()->set_address_taken(); - return true; - } + this->variable_->result_var_value()->set_address_taken(); else gcc_unreachable(); } @@ -1017,11 +993,10 @@ // need to know that they must live in the stack rather than in a // register. -bool -Temporary_reference_expression::do_address_taken(source_location, bool) +void +Temporary_reference_expression::do_address_taken(bool) { this->statement_->set_is_address_taken(); - return true; } // The temporary variable is being copied. We may need to increment @@ -2370,13 +2345,6 @@ do_copy() { return this; } - bool - do_address_taken(source_location location, bool escapes) - { - return this->constant_->const_value()->expr()->address_taken(location, - escapes); - } - Expression* do_being_copied(Refcounts*, bool); @@ -3600,7 +3568,8 @@ { return this->op_ == OPERATOR_MULT; } bool - do_address_taken(source_location, bool); + do_is_addressable() const + { return this->op_ == OPERATOR_MULT; } Expression* do_being_copied(Refcounts*, bool); @@ -4039,8 +4008,10 @@ break; case OPERATOR_AND: - if (!this->expr_->address_taken(this->location(), this->escapes_)) - this->set_is_error(); + if (!this->expr_->is_addressable()) + this->report_error(_("invalid operand for unary %<&%>")); + else + this->expr_->address_taken(this->escapes_); break; case OPERATOR_MULT: @@ -4058,17 +4029,6 @@ } } -// &*p is OK. - -bool -Unary_expression::do_address_taken(source_location location, bool) -{ - if (this->op_ == OPERATOR_MULT) - return true; - this->report_address_taken_error(location); - return false; -} - // Copying a unary expression may require incrementing a reference // count. @@ -8882,7 +8842,12 @@ { return this->end_ == NULL; } bool - do_address_taken(source_location, bool); + do_is_addressable() const + { return this->end_ == NULL; } + + void + do_address_taken(bool escapes) + { this->array_->address_taken(escapes); } Expression* do_being_copied(Refcounts*, bool); @@ -9015,25 +8980,6 @@ mpz_clear(lval); } -// We can take the address of an array index. We don't support taking -// the address of a slice--I'm not sure what the type of that would -// be. Taking the address of an array index implies taking the -// address of the array. - -bool -Array_index_expression::do_address_taken(source_location location, - bool escapes) -{ - if (!this->array_->address_taken(location, escapes)) - return false; - if (this->end_ != NULL) - { - error_at(location, "may not take address of array slice"); - return false; - } - return true; -} - // Copying an element of an array may require incrementing a reference // count. Copying a slice requires incrementing the reference count // of the underlying array. @@ -9299,10 +9245,6 @@ this->location()); } - bool - do_address_taken(source_location, bool) - { return true; } - Expression* do_being_copied(Refcounts*, bool); @@ -9588,20 +9530,6 @@ } } -// We can take the address of a map index expression if it is an -// lvalue. - -bool -Map_index_expression::do_address_taken(source_location location, bool) -{ - if (!this->is_lvalue_) - { - this->report_address_taken_error(location); - return false; - } - return true; -} - // If we are copying the map index to a variable, we need to increment // the reference count. @@ -9767,18 +9695,6 @@ gcc_assert(struct_type->field(this->field_index_) != NULL); } -// We can take the address of a field, and it implies taking the -// address of the whole structure. - -bool -Field_reference_expression::do_address_taken(source_location location, - bool escapes) -{ - if (!this->expr_->address_taken(location, escapes)) - return false; - return true; -} - // If we are copying the field to a variable, we need to increment the // reference count. @@ -10484,11 +10400,8 @@ } bool - do_address_taken(source_location, bool) - { - gcc_assert(this->is_constant_struct()); - return true; - } + do_is_addressable() const + { return true; } Expression* do_being_copied(Refcounts*, bool); @@ -10798,11 +10711,8 @@ do_check_types(Gogo*); bool - do_address_taken(source_location, bool) - { - gcc_assert(this->is_constant_array()); - return true; - } + do_is_addressable() const + { return true; } Expression* do_being_copied(Refcounts*, bool); @@ -11239,11 +11149,6 @@ this->location()); } - // Should be turned into Heap_composite_expression. - bool - do_address_taken(source_location, bool) - { gcc_unreachable(); } - Expression* do_being_copied(Refcounts*, bool); diff -r be235d7367a8 go/expressions.h --- a/go/expressions.h Tue Jun 22 10:39:31 2010 -0700 +++ b/go/expressions.h Tue Jun 22 11:29:31 2010 -0700 @@ -560,12 +560,17 @@ is_lvalue() const { return this->do_is_lvalue(); } - // We are taking the address of this expression. If this is - // invalid, report an error and return false. ESCAPES is true if - // this address escapes the current function. + // Return whether the expression is addressable--something which may + // be used as the operand of the unary & operator. bool - address_taken(source_location location, bool escapes) - { return this->do_address_taken(location, escapes); } + is_addressable() const + { return this->do_is_addressable(); } + + // Note that we are taking the address of this expression. ESCAPES + // is true if this address escapes the current function. + void + address_taken(bool escapes) + { this->do_address_taken(escapes); } // Return whether this expression must be evaluated in order // according to the order of evaluation rules. This is basically @@ -715,9 +720,15 @@ do_is_lvalue() const { return false; } + // Child class implements whether the expression is addressable. + virtual bool + do_is_addressable() const + { return false; } + // Child class implements taking the address of an expression. - virtual bool - do_address_taken(source_location, bool); + virtual void + do_address_taken(bool) + { } // Child class implements whether this expression must be evaluated // in order. @@ -760,10 +771,6 @@ void report_error(const char*); - // For errors when taking the address. - void - report_address_taken_error(source_location); - private: // Convert to the desired statement classification, or return NULL. // This is a controlled dynamic cast. @@ -956,7 +963,11 @@ { return true; } bool - do_address_taken(source_location, bool); + do_is_addressable() const + { return true; } + + void + do_address_taken(bool); Expression* do_being_copied(Refcounts*, bool); @@ -1000,7 +1011,11 @@ { return true; } bool - do_address_taken(source_location, bool); + do_is_addressable() const + { return true; } + + void + do_address_taken(bool); Expression* do_being_copied(Refcounts*, bool); @@ -1590,8 +1605,7 @@ do_is_lvalue() const { return this->is_lvalue_; } - bool - do_address_taken(source_location, bool); + // A map index expression is an lvalue but it is not addressable. Expression* do_being_copied(Refcounts*, bool); @@ -1743,7 +1757,8 @@ { return true; } bool - do_address_taken(source_location, bool); + do_is_addressable() const + { return this->expr_->is_addressable(); } Expression* do_being_copied(Refcounts*, bool); @@ -1819,10 +1834,6 @@ this->location()); } - bool - do_address_taken(source_location, bool) - { return true; } - tree do_get_tree(Translate_context*);