Go patch committed: Fix evaluation order of LHS index expressions

Message ID CAOyqgcUYa8NF8kULiUiAqm2BJJYYd-97jVxYLbdq93X0V5M16A@mail.gmail.com
State New
Headers show
Series
  • Go patch committed: Fix evaluation order of LHS index expressions
Related show

Commit Message

Ian Lance Taylor July 11, 2018, 2:22 p.m.
The Go 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. This patch adjusts the gofrontend code accordingly,
fixing the issue.  The test case for this is in
https://golang.org/cl/123115.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian

Patch

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 262540)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@ 
-8ad67a72a4fa59efffc891e73ecf10020e3c565d
+ea7ac7784791dca517b6681a02c39c11bf136755
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc	(revision 262540)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -10898,6 +10898,20 @@  Array_index_expression::do_check_types(G
     }
 }
 
+// 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*
Index: gcc/go/gofrontend/expressions.h
===================================================================
--- gcc/go/gofrontend/expressions.h	(revision 262540)
+++ gcc/go/gofrontend/expressions.h	(working copy)
@@ -2771,12 +2771,10 @@  class Index_expression : public Parser_e
 				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 Ex
   }
 
   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 E
   }
 
   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 Expr
   }
 
   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.