From patchwork Wed Sep 1 23:53:57 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: 63427 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 530EFB7157 for ; Thu, 2 Sep 2010 09:54:22 +1000 (EST) Received: (qmail 28751 invoked by alias); 1 Sep 2010 23:54:20 -0000 Received: (qmail 28742 invoked by uid 22791); 1 Sep 2010 23:54:18 -0000 X-SWARE-Spam-Status: No, hits=-2.0 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; Wed, 01 Sep 2010 23:54:08 +0000 Received: from hpaq14.eem.corp.google.com (hpaq14.eem.corp.google.com [172.25.149.14]) by smtp-out.google.com with ESMTP id o81Ns5DI011867 for ; Wed, 1 Sep 2010 16:54:06 -0700 Received: from pvg13 (pvg13.prod.google.com [10.241.210.141]) by hpaq14.eem.corp.google.com with ESMTP id o81Ns23u030270 for ; Wed, 1 Sep 2010 16:54:03 -0700 Received: by pvg13 with SMTP id 13so4243782pvg.24 for ; Wed, 01 Sep 2010 16:54:01 -0700 (PDT) Received: by 10.114.36.18 with SMTP id j18mr9552570waj.46.1283385241741; Wed, 01 Sep 2010 16:54:01 -0700 (PDT) Received: from coign.google.com ([66.109.106.2]) by mx.google.com with ESMTPS id k23sm19504260waf.17.2010.09.01.16.53.59 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 01 Sep 2010 16:54:00 -0700 (PDT) From: Ian Lance Taylor To: gcc-patches@gcc.gnu.org, gofrontend-dev@googlegroups.com Subject: [gccgo] Fix assignability and addressability tests to match spec Date: Wed, 01 Sep 2010 16:53:57 -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 fixes the tests in gccgo for assignability and addressability to match the language spec. Basically it rejects attempt to assign to non-addressable values. This fixes issue 237 in the Go issue tracker. Committed to gccgo branch. Ian diff -r 5310bc2fecb4 go/expressions.cc --- a/go/expressions.cc Tue Aug 31 21:35:15 2010 -0700 +++ b/go/expressions.cc Wed Sep 01 16:50:24 2010 -0700 @@ -886,10 +886,6 @@ { return this; } bool - do_is_lvalue() const - { return true; } - - bool do_is_addressable() const { return true; } @@ -1096,10 +1092,6 @@ do_copy() { return new Sink_expression(this->location()); } - bool - do_is_lvalue() const - { return true; } - tree do_get_tree(Translate_context*); @@ -3460,10 +3452,6 @@ } bool - do_is_lvalue() const - { return this->op_ == OPERATOR_MULT; } - - bool do_is_addressable() const { return this->op_ == OPERATOR_MULT; } @@ -3983,8 +3971,39 @@ return build_fold_addr_expr_loc(loc, expr); case OPERATOR_MULT: - gcc_assert(POINTER_TYPE_P(TREE_TYPE(expr))); - return build_fold_indirect_ref_loc(loc, expr); + { + gcc_assert(POINTER_TYPE_P(TREE_TYPE(expr))); + + // If we are dereferencing the pointer to a large struct, we + // need to check for nil. We don't bother to check for small + // structs because we expect the system to crash on a nil + // pointer dereference. + HOST_WIDE_INT s = int_size_in_bytes(TREE_TYPE(TREE_TYPE(expr))); + if (s == -1 || s >= 4096) + { + if (!DECL_P(expr)) + expr = save_expr(expr); + tree compare = fold_build2_loc(loc, EQ_EXPR, boolean_type_node, + expr, + fold_convert(TREE_TYPE(expr), + null_pointer_node)); + // FIXME: This should be a different error message. + static tree bad_index_fndecl; + tree crash = Gogo::call_builtin(&bad_index_fndecl, + loc, + "__go_bad_index", + 0, + void_type_node); + TREE_NOTHROW(bad_index_fndecl) = 0; + TREE_THIS_VOLATILE(bad_index_fndecl) = 1; + expr = fold_build2_loc(loc, COMPOUND_EXPR, TREE_TYPE(expr), + build3(COND_EXPR, void_type_node, + compare, crash, NULL_TREE), + expr); + } + + return build_fold_indirect_ref_loc(loc, expr); + } default: gcc_unreachable(); @@ -4054,6 +4073,21 @@ return new Unary_expression(op, expr, location); } +// If this is an indirection through a pointer, return the expression +// being pointed through. Otherwise return this. + +Expression* +Expression::deref() +{ + if (this->classification_ == EXPRESSION_UNARY) + { + Unary_expression* ue = static_cast(this); + if (ue->op() == OPERATOR_MULT) + return ue->operand(); + } + return this; +} + // Class Binary_expression. // Traversal. @@ -7865,10 +7899,10 @@ if (ue->op() == OPERATOR_MULT) { Field_reference_expression* fre = - ue->operand()->field_reference_expression(); + ue->operand()->deref()->field_reference_expression(); if (fre != NULL) { - Var_expression* ve = fre->expr()->var_expression(); + Var_expression* ve = fre->expr()->deref()->var_expression(); if (ve != NULL) { Named_object* no = ve->named_object(); @@ -8547,10 +8581,16 @@ Type* type = left->type(); if (type->is_error_type()) return Expression::make_error(location); - else if (type->array_type() != NULL - || (type->deref()->array_type() != NULL - && !type->deref()->array_type()->is_open_array_type())) + else if (type->array_type() != NULL) return Expression::make_array_index(left, start, end, location); + else if (type->points_to() != NULL + && type->points_to()->array_type() != NULL + && !type->points_to()->is_open_array_type()) + { + Expression* deref = Expression::make_unary(OPERATOR_MULT, left, + location); + return Expression::make_array_index(deref, start, end, location); + } else if (type->is_string_type()) return Expression::make_string_index(left, start, end, location); else if (type->map_type() != NULL) @@ -8619,12 +8659,7 @@ } bool - do_is_lvalue() const - { return this->end_ == NULL; } - - bool - do_is_addressable() const - { return this->end_ == NULL; } + do_is_addressable() const; void do_address_taken(bool escapes) @@ -8669,7 +8704,7 @@ { if (this->type_ == NULL) { - Array_type* type = this->array_->type()->deref()->array_type(); + Array_type* type = this->array_->type()->array_type(); if (type == NULL) this->type_ = Type::make_error_type(); else if (this->end_ == NULL) @@ -8713,7 +8748,7 @@ && !this->end_->is_nil_expression()) this->report_error(_("slice end must be integer")); - Array_type* array_type = this->array_->type()->deref()->array_type(); + Array_type* array_type = this->array_->type()->array_type(); gcc_assert(array_type != NULL); Type* dummy; @@ -8750,6 +8785,31 @@ } mpz_clear(ival); mpz_clear(lval); + + // A slice of an array requires an addressable array. A slice of a + // slice is always possible. + if (this->end_ != NULL + && !array_type->is_open_array_type() + && !this->array_->is_addressable()) + this->report_error(_("array is not addressable")); +} + +// Return whether this expression is addressable. + +bool +Array_index_expression::do_is_addressable() const +{ + // A slice expression is not addressable. + if (this->end_ != NULL) + return false; + + // An index into a slice is addressable. + if (this->array_->type()->is_open_array_type()) + return true; + + // An index into an array is addressable if the array is + // addressable. + return this->array_->is_addressable(); } // Get a tree for an array index. @@ -8760,11 +8820,7 @@ Gogo* gogo = context->gogo(); source_location loc = this->location(); - Type* t = this->array_->type(); - Type* pt = t->points_to(); - if (pt != NULL) - t = pt; - Array_type* array_type = t->array_type(); + Array_type* array_type = this->array_->type()->array_type(); gcc_assert(array_type != NULL); tree type_tree = array_type->get_tree(gogo); @@ -8774,17 +8830,6 @@ return error_mark_node; tree bad_index = boolean_false_node; - if (pt != NULL) - { - gcc_assert(!array_type->is_open_array_type()); - if (!DECL_P(array_tree)) - array_tree = save_expr(array_tree); - bad_index = build2(EQ_EXPR, boolean_type_node, array_tree, - fold_convert(TREE_TYPE(array_tree), - null_pointer_node)); - array_tree = build_fold_indirect_ref(array_tree); - } - tree start_tree = this->start_->get_tree(context); if (start_tree == error_mark_node) return error_mark_node; @@ -9334,11 +9379,7 @@ Type* Field_reference_expression::do_type() { - Type* expr_type = this->expr_->type(); - Type* points_to = expr_type->points_to(); - if (points_to != NULL) - expr_type = points_to; - Struct_type* struct_type = expr_type->struct_type(); + Struct_type* struct_type = this->expr_->type()->struct_type(); gcc_assert(struct_type != NULL); return struct_type->field(this->field_index_)->type(); } @@ -9348,11 +9389,7 @@ void Field_reference_expression::do_check_types(Gogo*) { - Type* expr_type = this->expr_->type(); - Type* points_to = expr_type->points_to(); - if (points_to != NULL) - expr_type = points_to; - Struct_type* struct_type = expr_type->struct_type(); + Struct_type* struct_type = this->expr_->type()->struct_type(); gcc_assert(struct_type != NULL); gcc_assert(struct_type->field(this->field_index_) != NULL); } @@ -9366,38 +9403,6 @@ if (struct_tree == error_mark_node || TREE_TYPE(struct_tree) == error_mark_node) return error_mark_node; - - if (POINTER_TYPE_P(TREE_TYPE(struct_tree))) - { - // If we are dereferencing the pointer to a large struct, we - // need to check for nil. We don't bother to check for small - // structs because we expect the system to crash on a nil - // pointer dereference. - HOST_WIDE_INT s = int_size_in_bytes(TREE_TYPE(TREE_TYPE(struct_tree))); - if (s == -1 || s >= 4096) - { - if (!DECL_P(struct_tree)) - struct_tree = save_expr(struct_tree); - tree compare = build2(EQ_EXPR, boolean_type_node, struct_tree, - fold_convert(TREE_TYPE(struct_tree), - null_pointer_node)); - // FIXME: This should be a different error message. - static tree bad_index_fndecl; - tree crash = Gogo::call_builtin(&bad_index_fndecl, - this->location(), - "__go_bad_index", - 0, - void_type_node); - TREE_NOTHROW(bad_index_fndecl) = 0; - TREE_THIS_VOLATILE(bad_index_fndecl) = 1; - struct_tree = build2(COMPOUND_EXPR, TREE_TYPE(struct_tree), - build3(COND_EXPR, void_type_node, compare, - crash, NULL_TREE), - struct_tree); - } - struct_tree = build_fold_indirect_ref(struct_tree); - } - gcc_assert(TREE_CODE(TREE_TYPE(struct_tree)) == RECORD_TYPE); tree field = TYPE_FIELDS(TREE_TYPE(struct_tree)); gcc_assert(field != NULL_TREE); diff -r 5310bc2fecb4 go/expressions.h --- a/go/expressions.h Tue Aug 31 21:35:15 2010 -0700 +++ b/go/expressions.h Wed Sep 01 16:50:24 2010 -0700 @@ -376,6 +376,11 @@ is_nil_expression() const { return this->classification_ == EXPRESSION_NIL; } + // If this is an indirection through a pointer, return the + // expression being pointed through. Otherwise return this. + Expression* + deref(); + // If this is a binary expression, return the Binary_expression // structure. Otherwise return NULL. Binary_expression* @@ -517,12 +522,6 @@ copy() { return this->do_copy(); } - // Return whether the expression is an lvalue--something which may - // appear on the left hand side of an assignment statement. - bool - is_lvalue() const - { return this->do_is_lvalue(); } - // Return whether the expression is addressable--something which may // be used as the operand of the unary & operator. bool @@ -650,11 +649,6 @@ virtual Expression* do_copy() = 0; - // Child class implements whether the expression is an lvalue. - virtual bool - do_is_lvalue() const - { return false; } - // Child class implements whether the expression is addressable. virtual bool do_is_addressable() const @@ -891,10 +885,6 @@ { return this; } bool - do_is_lvalue() const - { return true; } - - bool do_is_addressable() const { return true; } @@ -933,10 +923,6 @@ { return make_temporary_reference(this->statement_, this->location()); } bool - do_is_lvalue() const - { return true; } - - bool do_is_addressable() const { return true; } @@ -1493,10 +1479,6 @@ this->location()); } - bool - do_is_lvalue() const - { return this->is_lvalue_; } - // A map index expression is an lvalue but it is not addressable. tree @@ -1636,10 +1618,6 @@ } bool - do_is_lvalue() const - { return true; } - - bool do_is_addressable() const { return this->expr_->is_addressable(); } @@ -1648,7 +1626,7 @@ private: // The expression we are looking into. This should have a type of - // struct or pointer to struct. + // struct. Expression* expr_; // The zero-based index of the field we are retrieving. unsigned int field_index_; diff -r 5310bc2fecb4 go/parse.cc --- a/go/parse.cc Tue Aug 31 21:35:15 2010 -0700 +++ b/go/parse.cc Wed Sep 01 16:50:24 2010 -0700 @@ -2265,6 +2265,7 @@ Expression* closure_ref = Expression::make_var_reference(closure, location); + closure_ref = Expression::make_unary(OPERATOR_MULT, closure_ref, location); // The closure structure holds pointers to the variables, so we need // to introduce an indirection. diff -r 5310bc2fecb4 go/statements.cc --- a/go/statements.cc Tue Aug 31 21:35:15 2010 -0700 +++ b/go/statements.cc Wed Sep 01 16:50:24 2010 -0700 @@ -473,7 +473,11 @@ void Assignment_statement::do_check_types(Gogo*) { - if (!this->lhs_->is_lvalue()) + // The left hand side must be either addressable, a map index + // expression, or the blank identifier. + if (!this->lhs_->is_addressable() + && this->lhs_->map_index_expression() == NULL + && !this->lhs_->is_sink_expression()) { if (!this->lhs_->type()->is_error_type()) this->report_error(_("invalid left hand side of assignment")); @@ -2102,6 +2106,8 @@ // ones used in build_struct. Expression* thunk_parameter = Expression::make_var_reference(named_parameter, location); + thunk_parameter = Expression::make_unary(OPERATOR_MULT, thunk_parameter, + location); Bound_method_expression* bound_method = ce->fn()->bound_method_expression(); Interface_field_reference_expression* interface_method = @@ -2151,6 +2157,8 @@ { Expression* thunk_param = Expression::make_var_reference(named_parameter, location); + thunk_param = Expression::make_unary(OPERATOR_MULT, thunk_param, + location); Expression* param = Expression::make_field_reference(thunk_param, next_index, location); diff -r 5310bc2fecb4 go/types.cc --- a/go/types.cc Tue Aug 31 21:35:15 2010 -0700 +++ b/go/types.cc Wed Sep 01 16:50:24 2010 -0700 @@ -2925,9 +2925,11 @@ found_depth = subdepth; Expression* here = Expression::make_field_reference(struct_expr, i, location); + if (pf->type()->points_to() != NULL) + here = Expression::make_unary(OPERATOR_MULT, here, location); while (sub->expr() != NULL) { - sub = sub->expr()->field_reference_expression(); + sub = sub->expr()->deref()->field_reference_expression(); gcc_assert(sub != NULL); } sub->set_struct_expression(here); @@ -6239,6 +6241,12 @@ Struct_type* stype = expr->type()->deref()->struct_type(); gcc_assert(stype != NULL && field_indexes->field_index < stype->field_count()); + if (expr->type()->struct_type() == NULL) + { + gcc_assert(expr->type()->points_to() != NULL); + expr = Expression::make_unary(OPERATOR_MULT, expr, location); + gcc_assert(expr->type()->struct_type() == stype); + } return Expression::make_field_reference(expr, field_indexes->field_index, location); } @@ -6303,7 +6311,7 @@ const Interface_type* it = type->deref()->interface_type(); bool receiver_can_be_pointer = (expr->type()->points_to() != NULL - || expr->is_lvalue()); + || expr->is_addressable()); bool is_method = false; bool found_pointer_method = false; std::string ambig1; @@ -6316,6 +6324,13 @@ if (!is_method) { gcc_assert(st != NULL); + if (type->struct_type() == NULL) + { + gcc_assert(type->points_to() != NULL); + expr = Expression::make_unary(OPERATOR_MULT, expr, + location); + gcc_assert(expr->type()->struct_type() == st); + } ret = st->field_reference(expr, name, location); } else if (it != NULL && it->find_method(name) != NULL)