Patchwork Go patch committed: len(<-c) is not a constant

login
register
mail settings
Submitter Ian Taylor
Date Sept. 21, 2012, 5:48 p.m.
Message ID <mcrobkzp8ql.fsf@google.com>
Download mbox | patch
Permalink /patch/185859/
State New
Headers show

Comments

Ian Taylor - Sept. 21, 2012, 5:48 p.m.
The Go compiler was erroneously treating len(<-c) as a constant if c was
a chan of an array.  That was causing it to not actually run the channel
receive operation.  This patch fixes the problem, and also does a minor
cleanup by moving the relevant code from do_lower to do_is_constant.
Bootstrapped and tested on x86_64-unknown-linux-gnu.  Committed to
mainline and 4.7 branch.

Ian

Patch

diff -r 0776ddf9dcf5 go/expressions.cc
--- a/go/expressions.cc	Thu Sep 20 10:06:54 2012 -0700
+++ b/go/expressions.cc	Fri Sep 21 10:43:39 2012 -0700
@@ -6679,38 +6679,6 @@ 
   this->set_args(new_args);
 }
 
-// A traversal class which looks for a call expression.
-
-class Find_call_expression : public Traverse
-{
- public:
-  Find_call_expression()
-    : Traverse(traverse_expressions),
-      found_(false)
-  { }
-
-  int
-  expression(Expression**);
-
-  bool
-  found()
-  { return this->found_; }
-
- private:
-  bool found_;
-};
-
-int
-Find_call_expression::expression(Expression** pexpr)
-{
-  if ((*pexpr)->call_expression() != NULL)
-    {
-      this->found_ = true;
-      return TRAVERSE_EXIT;
-    }
-  return TRAVERSE_CONTINUE;
-}
-
 // Lower a builtin call expression.  This turns new and make into
 // specific expressions.  We also convert to a constant if we can.
 
@@ -6731,20 +6699,6 @@ 
 
   if (this->is_constant())
     {
-      // We can only lower len and cap if there are no function calls
-      // in the arguments.  Otherwise we have to make the call.
-      if (this->code_ == BUILTIN_LEN || this->code_ == BUILTIN_CAP)
-	{
-	  Expression* arg = this->one_arg();
-	  if (arg != NULL && !arg->is_constant())
-	    {
-	      Find_call_expression find_call;
-	      Expression::traverse(&arg, &find_call);
-	      if (find_call.found())
-		return this;
-	    }
-	}
-
       Numeric_constant nc;
       if (this->numeric_constant_value(&nc))
 	return nc.expression(loc);
@@ -7061,8 +7015,42 @@ 
   return args->front();
 }
 
-// Return whether this is constant: len of a string, or len or cap of
-// a fixed array, or unsafe.Sizeof, unsafe.Offsetof, unsafe.Alignof.
+// A traversal class which looks for a call or receive expression.
+
+class Find_call_expression : public Traverse
+{
+ public:
+  Find_call_expression()
+    : Traverse(traverse_expressions),
+      found_(false)
+  { }
+
+  int
+  expression(Expression**);
+
+  bool
+  found()
+  { return this->found_; }
+
+ private:
+  bool found_;
+};
+
+int
+Find_call_expression::expression(Expression** pexpr)
+{
+  if ((*pexpr)->call_expression() != NULL
+      || (*pexpr)->receive_expression() != NULL)
+    {
+      this->found_ = true;
+      return TRAVERSE_EXIT;
+    }
+  return TRAVERSE_CONTINUE;
+}
+
+// Return whether this is constant: len of a string constant, or len
+// or cap of an array, or unsafe.Sizeof, unsafe.Offsetof,
+// unsafe.Alignof.
 
 bool
 Builtin_call_expression::do_is_constant() const
@@ -7085,6 +7073,17 @@ 
 	    && !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)
 	  return true;