commit ea7ac7784791dca517b6681a02c39c11bf136755
author Ian Lance Taylor <iant@golang.org> Tue Jul 10 14:31:36 2018 -0700
committer Ian Lance Taylor <iant@golang.org> Wed Jul 11 14:20:55 2018 +0000
tree 3538ad2540270b56b44b7d24582991c6c66de50d
parent 8ad67a72a4fa59efffc891e73ecf10020e3c565d

compiler: fix evaluation order of LHS index expressions

The spec says that when an index expression appears on the left hand
side of an assignment, the operands should be evaluated. The
gofrontend code was assuming that that only referred to the index
operand. But discussion of https://golang.org/issue/23188 has
clarified that this means both the slice/map/string operand and the
index operand. Adjust the gofrontend code accordingly.

Fixes golang/go#23188

Change-Id: I90e17ada43df58d439c060344f9224dbe1e7dacd
Reviewed-on: https://go-review.googlesource.com/123155
Reviewed-by: Than McIntosh <thanm@google.com>

go/expressions.cc

diff --git a/go/expressions.cc b/go/expressions.cc
index 89b265b..e7d0b75 100644
--- a/go/expressions.cc
+++ b/go/expressions.cc
@@ -10898,6 +10898,20 @@
     }
 }
 
+// The subexpressions of an array index must be evaluated in order.
+// If this is indexing into an array, rather than a slice, then only
+// the index should be evaluated.  Since this is called for values on
+// the left hand side of an assigment, evaluating the array, meaning
+// copying the array, will cause a different array to be modified.
+
+bool
+Array_index_expression::do_must_eval_subexpressions_in_order(
+    int* skip) const
+{
+  *skip = this->array_->type()->is_slice_type() ? 0 : 1;
+  return true;
+}
+
 // Flatten array indexing by using temporary variables for slices and indexes.
 
 Expression*

go/expressions.h

diff --git a/go/expressions.h b/go/expressions.h
index a58c79c..5fa4171 100644
--- a/go/expressions.h
+++ b/go/expressions.h
@@ -2771,12 +2771,10 @@
 				this->location());
   }
 
+  // This shouldn't be called--we don't know yet.
   bool
-  do_must_eval_subexpressions_in_order(int* skip) const
-  {
-    *skip = 1;
-    return true;
-  }
+  do_must_eval_subexpressions_in_order(int*) const
+  { go_unreachable(); }
 
   void
   do_dump_expression(Ast_dump_context*) const;
@@ -2882,11 +2880,7 @@
   }
 
   bool
-  do_must_eval_subexpressions_in_order(int* skip) const
-  {
-    *skip = 1;
-    return true;
-  }
+  do_must_eval_subexpressions_in_order(int* skip) const;
 
   bool
   do_is_addressable() const;
@@ -2965,11 +2959,8 @@
   }
 
   bool
-  do_must_eval_subexpressions_in_order(int* skip) const
-  {
-    *skip = 1;
-    return true;
-  }
+  do_must_eval_subexpressions_in_order(int*) const
+  { return true; }
 
   Bexpression*
   do_get_backend(Translate_context*);
@@ -3052,11 +3043,8 @@
   }
 
   bool
-  do_must_eval_subexpressions_in_order(int* skip) const
-  {
-    *skip = 1;
-    return true;
-  }
+  do_must_eval_subexpressions_in_order(int*) const
+  { return true; }
 
   // A map index expression is an lvalue but it is not addressable.