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 <[email protected]>
golang · Jul 11, 2018 · ea7ac77 · ea7ac77
1 parent 8ad67a7
commit ea7ac77
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 20 deletions.
diff --git a/go/expressions.cc b/go/expressions.cc
@@ -10898,6 +10898,20 @@ Array_index_expression::do_check_types(Gogo*)
     }
 }
 
+// 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*

diff --git a/go/expressions.h b/go/expressions.h
@@ -2771,12 +2771,10 @@ class Index_expression : public Parser_expression
 				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 @@ class Array_index_expression : public Expression
   }
 
   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 @@ class String_index_expression : public Expression
   }
 
   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 @@ class Map_index_expression : public Expression
   }
 
   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.