Patchwork [gccgo] Compare interface values with non-interface values

login
register
mail settings
Submitter Ian Taylor
Date Nov. 11, 2010, 1:54 a.m.
Message ID <mcrr5estyj9.fsf@google.com>
Download mbox | patch
Permalink /patch/70752/
State New
Headers show

Comments

Ian Taylor - Nov. 11, 2010, 1:54 a.m.
Go permits comparison interface values with non-interface values, but I
somehow failed to implement it.  This patch fixes that oversight, and
uses the correct type checking rules for comparisons.  Committed to
gccgo branch.

Ian

Patch

diff -r 02dcc12e55f9 go/expressions.cc
--- a/go/expressions.cc	Wed Nov 10 16:50:19 2010 -0800
+++ b/go/expressions.cc	Wed Nov 10 17:50:47 2010 -0800
@@ -5461,49 +5461,67 @@ 
 void
 Binary_expression::do_check_types(Gogo*)
 {
-  if (this->op_ != OPERATOR_LSHIFT && this->op_ != OPERATOR_RSHIFT)
-    {
-      Type* type = this->left_->type();
-
-      if (!Type::are_compatible_for_binop(type, this->right_->type()))
+  Type* left_type = this->left_->type();
+  Type* right_type = this->right_->type();
+  if (left_type->is_error_type() || right_type->is_error_type())
+    return;
+
+  if (this->op_ == OPERATOR_EQEQ
+      || this->op_ == OPERATOR_NOTEQ
+      || this->op_ == OPERATOR_LT
+      || this->op_ == OPERATOR_LE
+      || this->op_ == OPERATOR_GT
+      || this->op_ == OPERATOR_GE)
+    {
+      if (!Type::are_assignable(left_type, right_type, NULL)
+	  && !Type::are_assignable(right_type, left_type, NULL))
 	{
 	  this->report_error(_("incompatible types in binary expression"));
 	  return;
 	}
-
-      if (!type->is_error_type()
-	  && !this->right_->type()->is_error_type())
-	{
-	  if (!Binary_expression::check_operator_type(this->op_, type,
-						      this->location()))
-	    this->set_is_error();
+      if (!Binary_expression::check_operator_type(this->op_, left_type,
+						  this->location())
+	  || !Binary_expression::check_operator_type(this->op_, right_type,
+						     this->location()))
+	{
+	  this->set_is_error();
+	  return;
+	}
+    }
+  else if (this->op_ != OPERATOR_LSHIFT && this->op_ != OPERATOR_RSHIFT)
+    {
+      if (!Type::are_compatible_for_binop(left_type, right_type))
+	{
+	  this->report_error(_("incompatible types in binary expression"));
+	  return;
+	}
+      if (!Binary_expression::check_operator_type(this->op_, left_type,
+						  this->location()))
+	{
+	  this->set_is_error();
+	  return;
 	}
     }
   else
     {
-      if (this->left_->type()->integer_type() == NULL
-	  && !this->left_->type()->is_error_type())
+      if (left_type->integer_type() == NULL)
 	this->report_error(_("shift of non-integer operand"));
 
-      Type* shift_type = this->right_->type();
-      if (!shift_type->is_error_type())
-	{
-	  if (!shift_type->is_abstract()
-	      && (shift_type->integer_type() == NULL
-		  || !shift_type->integer_type()->is_unsigned()))
-	    this->report_error(_("shift count not unsigned integer"));
-	  else
+      if (!right_type->is_abstract()
+	  && (right_type->integer_type() == NULL
+	      || !right_type->integer_type()->is_unsigned()))
+	this->report_error(_("shift count not unsigned integer"));
+      else
+	{
+	  mpz_t val;
+	  mpz_init(val);
+	  Type* type;
+	  if (this->right_->integer_constant_value(true, val, &type))
 	    {
-	      mpz_t val;
-	      mpz_init(val);
-	      Type* type;
-	      if (this->right_->integer_constant_value(true, val, &type))
-		{
-		  if (mpz_sgn(val) < 0)
-		    this->report_error(_("negative shift count"));
-		}
-	      mpz_clear(val);
+	      if (mpz_sgn(val) < 0)
+		this->report_error(_("negative shift count"));
 	    }
+	  mpz_clear(val);
 	}
     }
 }
@@ -5929,8 +5947,93 @@ 
       right_tree = build_int_cst_type(integer_type_node, 0);
     }
 
-  if (left_type->interface_type() != NULL
-      && right_type->interface_type() != NULL)
+  if ((left_type->interface_type() != NULL
+       && right_type->interface_type() == NULL
+       && !right_type->is_nil_type())
+      || (left_type->interface_type() == NULL
+	  && !left_type->is_nil_type()
+	  && right_type->interface_type() != NULL))
+    {
+      // Comparing an interface value to a non-interface value.
+      if (left_type->interface_type() == NULL)
+	{
+	  std::swap(left_type, right_type);
+	  std::swap(left_tree, right_tree);
+	}
+
+      // The right operand is not an interface.  We need to take its
+      // address if it is not a pointer.
+      tree make_tmp;
+      tree arg;
+      if (right_type->points_to() != NULL)
+	{
+	  make_tmp = NULL_TREE;
+	  arg = right_tree;
+	}
+      else if (TREE_ADDRESSABLE(TREE_TYPE(right_tree)) || DECL_P(right_tree))
+	{
+	  make_tmp = NULL_TREE;
+	  arg = build_fold_addr_expr_loc(location, right_tree);
+	  if (DECL_P(right_tree))
+	    TREE_ADDRESSABLE(right_tree) = 1;
+	}
+      else
+	{
+	  tree tmp = create_tmp_var(TREE_TYPE(right_tree),
+				    get_name(right_tree));
+	  DECL_IGNORED_P(tmp) = 0;
+	  DECL_INITIAL(tmp) = right_tree;
+	  TREE_ADDRESSABLE(tmp) = 1;
+	  make_tmp = build1(DECL_EXPR, void_type_node, tmp);
+	  SET_EXPR_LOCATION(make_tmp, location);
+	  arg = build_fold_addr_expr_loc(location, tmp);
+	}
+      arg = fold_convert_loc(location, ptr_type_node, arg);
+
+      tree descriptor = right_type->type_descriptor_pointer(context->gogo());
+
+      if (left_type->interface_type()->is_empty())
+	{
+	  static tree empty_interface_value_compare_decl;
+	  left_tree = Gogo::call_builtin(&empty_interface_value_compare_decl,
+					 location,
+					 "__go_empty_interface_value_compare",
+					 3,
+					 integer_type_node,
+					 TREE_TYPE(left_tree),
+					 left_tree,
+					 TREE_TYPE(descriptor),
+					 descriptor,
+					 ptr_type_node,
+					 arg);
+	  // This can panic if the type is not comparable.
+	  TREE_NOTHROW(empty_interface_value_compare_decl) = 0;
+	}
+      else
+	{
+	  static tree interface_value_compare_decl;
+	  left_tree = Gogo::call_builtin(&interface_value_compare_decl,
+					 location,
+					 "__go_interface_value_compare",
+					 3,
+					 integer_type_node,
+					 TREE_TYPE(left_tree),
+					 left_tree,
+					 TREE_TYPE(descriptor),
+					 descriptor,
+					 ptr_type_node,
+					 arg);
+	  // This can panic if the type is not comparable.
+	  TREE_NOTHROW(interface_value_compare_decl) = 0;
+	}
+      right_tree = build_int_cst_type(integer_type_node, 0);
+
+      if (make_tmp != NULL_TREE)
+	left_tree = build2(COMPOUND_EXPR, TREE_TYPE(left_tree), make_tmp,
+			   left_tree);
+    }
+  else if (left_type->interface_type() != NULL
+	   && right_type->interface_type() != NULL)
     {
       if (left_type->interface_type()->is_empty())
 	{
@@ -5947,7 +6050,6 @@ 
 					 right_tree);
 	  // This can panic if the type is uncomparable.
 	  TREE_NOTHROW(empty_interface_compare_decl) = 0;
-	  right_tree = build_int_cst_type(integer_type_node, 0);
 	}
       else
 	{
@@ -5964,8 +6066,8 @@ 
 					 right_tree);
 	  // This can panic if the type is uncomparable.
 	  TREE_NOTHROW(interface_compare_decl) = 0;
-	  right_tree = build_int_cst_type(integer_type_node, 0);
-	}
+	}
+      right_tree = build_int_cst_type(integer_type_node, 0);
     }
 
   if (left_type->is_nil_type()
diff -r 02dcc12e55f9 go/statements.cc
--- a/go/statements.cc	Wed Nov 10 16:50:19 2010 -0800
+++ b/go/statements.cc	Wed Nov 10 17:50:47 2010 -0800
@@ -3077,7 +3077,8 @@ 
 	   p != this->cases_->end();
 	   ++p)
 	{
-	  if (!Type::are_compatible_for_binop(type, (*p)->type()))
+	  if (!Type::are_assignable(type, (*p)->type(), NULL)
+	      && !Type::are_assignable((*p)->type(), type, NULL))
 	    {
 	      error_at((*p)->location(),
 		       "type mismatch between switch value and case clause");
diff -r 02dcc12e55f9 go/types.cc
--- a/go/types.cc	Wed Nov 10 16:50:19 2010 -0800
+++ b/go/types.cc	Wed Nov 10 17:50:47 2010 -0800
@@ -394,7 +394,7 @@ 
 }
 
 // Return true if it's OK to have a binary operation with types LHS
-// and RHS.  This is not used for shifts.
+// and RHS.  This is not used for shifts or comparisons.
 
 bool
 Type::are_compatible_for_binop(const Type* lhs, const Type* rhs)
diff -r 02dcc12e55f9 go/types.h
--- a/go/types.h	Wed Nov 10 16:50:19 2010 -0800
+++ b/go/types.h	Wed Nov 10 17:50:47 2010 -0800
@@ -512,7 +512,8 @@ 
   are_identical(const Type* lhs, const Type* rhs, std::string* reason);
 
   // Return true if two types are compatible for use in a binary
-  // operation.  This is an equivalence relation.
+  // operation, other than a shift, comparison, or channel send.  This
+  // is an equivalence relation.
   static bool
   are_compatible_for_binop(const Type* t1, const Type* t2);
 
diff -r 02dcc12e55f9 libgo/runtime/go-eface-val-compare.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/libgo/runtime/go-eface-val-compare.c	Wed Nov 10 17:50:47 2010 -0800
@@ -0,0 +1,32 @@ 
+/* go-eface-val-compare.c -- compare an empty interface with a value.
+
+   Copyright 2010 The Go Authors. All rights reserved.
+   Use of this source code is governed by a BSD-style
+   license that can be found in the LICENSE file.  */
+
+#include "go-type.h"
+#include "interface.h"
+
+/* Compare an empty interface with a value.  Return 0 for equal, not
+   zero for not equal (return value is like strcmp).  */
+
+int
+__go_empty_interface_value_compare (
+    struct __go_empty_interface left,
+    const struct __go_type_descriptor *right_descriptor,
+    const void *val)
+{
+  const struct __go_type_descriptor *left_descriptor;
+
+  left_descriptor = left.__type_descriptor;
+  if (left_descriptor == NULL)
+    return 1;
+  if (!__go_type_descriptors_equal (left_descriptor, right_descriptor))
+    return 1;
+  if (__go_is_pointer_type (left_descriptor))
+    return left.__object == val ? 0 : 1;
+  if (!left_descriptor->__equalfn (left.__object, val,
+				   left_descriptor->__size))
+    return 1;
+  return 0;
+}
diff -r 02dcc12e55f9 libgo/runtime/go-interface-val-compare.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/libgo/runtime/go-interface-val-compare.c	Wed Nov 10 17:50:47 2010 -0800
@@ -0,0 +1,32 @@ 
+/* go-interface-val-compare.c -- compare an interface to a value.
+
+   Copyright 2009 The Go Authors. All rights reserved.
+   Use of this source code is governed by a BSD-style
+   license that can be found in the LICENSE file.  */
+
+#include "go-type.h"
+#include "interface.h"
+
+/* Compare two interface values.  Return 0 for equal, not zero for not
+   equal (return value is like strcmp).  */
+
+int
+__go_interface_value_compare (
+    struct __go_interface left,
+    const struct __go_type_descriptor *right_descriptor,
+    const void *val)
+{
+  const struct __go_type_descriptor *left_descriptor;
+
+  if (left.__methods == NULL)
+    return 1;
+  left_descriptor = left.__methods[0];
+  if (!__go_type_descriptors_equal (left_descriptor, right_descriptor))
+    return 1;
+  if (__go_is_pointer_type (left_descriptor))
+    return left.__object == val ? 0 : 1;
+  if (!left_descriptor->__equalfn (left.__object, val,
+				   left_descriptor->__size))
+    return 1;
+  return 0;
+}