Patchwork [gccgo] Fix assignability and addressability tests to match spec

login
register
mail settings
Submitter Ian Taylor
Date Sept. 1, 2010, 11:53 p.m.
Message ID <mcrzkw16mwa.fsf@google.com>
Download mbox | patch
Permalink /patch/63427/
State New
Headers show

Comments

Ian Taylor - Sept. 1, 2010, 11:53 p.m.
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

Patch

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<Unary_expression*>(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)