From 8fdb25dab807d46e8bfda10eb03003799943ccca Mon Sep 17 00:00:00 2001
From: Andy Chu <andy@oilshell.org>
Date: Sun, 8 Mar 2020 12:03:45 -0700
Subject: [PATCH] [spec/arith] Failing cases for dynamic LHS name in arith
 assignment.

Addresses issue #640.
---
 frontend/syntax.asdl |  2 ++
 osh/arith_parse.py   |  4 +++-
 osh/sh_expr_eval.py  | 16 +++++++-------
 osh/tdop.py          |  6 ++++-
 spec/arith.test.sh   | 52 ++++++++++++++++++++++++++++++++++++++++++++
 test/spec.sh         |  2 +-
 6 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/frontend/syntax.asdl b/frontend/syntax.asdl
index 3a33f57baf..b1e233526d 100644
--- a/frontend/syntax.asdl
+++ b/frontend/syntax.asdl
@@ -221,6 +221,8 @@ module syntax
     VarRef(Token token)  -- variable without $
   | ArithWord(word w)  -- a string that looks like an integer
 
+    -- TODO: instead of sh_lhs_expr, could be arith_expr with
+    -- VarRef|ArithWord|Binary for a[]
   | UnaryAssign(id op_id, sh_lhs_expr child)
   | BinaryAssign(id op_id, sh_lhs_expr left, arith_expr right)
   | Unary(id op_id, arith_expr child)
diff --git a/osh/arith_parse.py b/osh/arith_parse.py
index 3811f44c43..cf62899293 100755
--- a/osh/arith_parse.py
+++ b/osh/arith_parse.py
@@ -51,6 +51,8 @@ def LeftIncDec(p, w, left, rbp):
     raise AssertionError()
 
   child = tdop.ToLValue(left)
+  if child is None:
+    p_die("This value can't be assigned to", word=w)
   return arith_expr.UnaryAssign(op_id, child)
 
 
@@ -75,7 +77,7 @@ def LeftIndex(p, w, left, unused_bp):
   3. strings don't have mutable characters.
   """
   if not tdop.IsIndexable(left):
-    p_die("The [ operarator doesn't apply to this expression", word=w)
+    p_die("The [ operator doesn't apply to this expression", word=w)
   index = p.ParseUntil(0)
   p.Eat(Id.Arith_RBracket)
 
diff --git a/osh/sh_expr_eval.py b/osh/sh_expr_eval.py
index 3f675338e2..6f83c6e2c5 100755
--- a/osh/sh_expr_eval.py
+++ b/osh/sh_expr_eval.py
@@ -124,17 +124,17 @@ def _StringToInteger(s, span_id=runtime.NO_SPID):
 # Common logic for Arith and Command/Word variants of the same expression
 #
 # Calls EvalLhs()
-#   a[$key]=$val                 # osh/cmd_exec.py:814  (command_e.ShAssignment)
+#   a[$key]=$val             # osh/cmd_exec.py:814  (command_e.ShAssignment)
 # Calls _EvalLhsArith()
-#   (( a[key] = val ))           # osh/expr_eval.py:326 (_EvalLhsArith)
+#   (( a[key] = val ))       # osh/sh_expr_eval.py:326 (_EvalLhsArith)
 #
 # Calls EvalLhsAndLookup():
-#   a[$key]+=$val                # osh/cmd_exec.py:795  (assign_op_e.PlusEqual)
-#   (( a[key] += val ))          # osh/expr_eval.py:308 (_EvalLhsAndLookupArith)
+#   a[$key]+=$val            # osh/cmd_exec.py:795     (assign_op_e.PlusEqual)
+#   (( a[key] += val ))      # osh/sh_expr_eval.py:308 (_EvalLhsAndLookupArith)
 #
 # Uses Python's [] operator
-#   val=${a[$key]}               # osh/word_eval.py:639 (bracket_op_e.ArrayIndex)
-#   (( val = a[key] ))           # osh/expr_eval.py:509 (Id.Arith_LBracket)
+#   val=${a[$key]}           # osh/word_eval.py:639 (bracket_op_e.ArrayIndex)
+#   (( val = a[key] ))       # osh/sh_expr_eval.py:509 (Id.Arith_LBracket)
 #
 
 
@@ -191,9 +191,9 @@ def EvalLhs(node, arith_ev, mem, spid, lookup_mode):
 
 def _EvalLhsArith(node, mem, arith_ev):
   # type: (sh_lhs_expr_t, Mem, ArithEvaluator) -> lvalue_t
-  """sh_lhs_expr -> lvalue.
+  """Evaluate LHS for arithmetic.
   
-  Very similar to EvalLhs above in core/cmd_exec.
+  Very similar to EvalLhs above, called in osh/cmd_exec.py.
   """
   assert isinstance(node, sh_lhs_expr_t), node
 
diff --git a/osh/tdop.py b/osh/tdop.py
index 4c15bda691..5c423c93dd 100644
--- a/osh/tdop.py
+++ b/osh/tdop.py
@@ -5,7 +5,7 @@
 from _devbuild.gen.id_kind_asdl import Id, Id_t
 from _devbuild.gen.syntax_asdl import (
     arith_expr, arith_expr_e, arith_expr_t,
-    arith_expr__VarRef, arith_expr__Binary,
+    arith_expr__VarRef, arith_expr__Binary, arith_expr__ArithWord,
     sh_lhs_expr, sh_lhs_expr_t,
     word_t,
 )
@@ -30,8 +30,11 @@ def IsIndexable(node):
   # type: (arith_expr_t) -> bool
   """
   a[1] is allowed but a[1][1] isn't
+
   """
   return node.tag_() == arith_expr_e.VarRef
+  # TODO: x$foo[1] is also allowed
+  #return node.tag_() in (arith_expr_e.VarRef, arith_expr_e.ArithWord)
 
 
 def ToLValue(node):
@@ -50,6 +53,7 @@ def ToLValue(node):
       node = cast(arith_expr__VarRef, UP_node)
       # For consistency with osh/cmd_parse.py, append a span_id.
       # TODO: (( a[ x ] = 1 )) and a[x]=1 should use different LST nodes.
+      # sh_lhs_expr should be an "IR".
       n = sh_lhs_expr.Name(node.token.val)
       n.spids.append(node.token.span_id)
       return n
diff --git a/spec/arith.test.sh b/spec/arith.test.sh
index b6c7d19708..5bad5afebc 100644
--- a/spec/arith.test.sh
+++ b/spec/arith.test.sh
@@ -479,3 +479,55 @@ last=6
 ## N-I dash stdout-json: ""
 ## BUG zsh status: 1
 ## BUG zsh stdout-json: ""
+
+#### assignment with dynamic var name
+foo=bar
+echo $(( x$foo = 42 ))
+echo xbar=$xbar
+## STDOUT:
+42
+xbar=42
+## END
+
+#### array assignment with dynamic array name
+foo=bar
+echo $(( x$foo[5] = 42 ))
+echo 'xbar[5]='${xbar[5]}
+## STDOUT:
+42
+xbar[5]=42
+## END
+## BUG zsh STDOUT:
+42
+xbar[5]=
+## END
+## N-I dash status: 2
+## N-I dash stdout-json: ""
+
+#### unary assignment with dynamic var name
+foo=bar
+xbar=42
+echo $(( x$foo++ ))
+echo xbar=$xbar
+## STDOUT:
+42
+xbar=43
+## END
+## BUG dash status: 2
+## BUG dash stdout-json: ""
+
+#### unary array assignment with dynamic var name
+foo=bar
+xbar[5]=42
+echo $(( x$foo[5]++ ))
+echo 'xbar[5]='${xbar[5]}
+## STDOUT:
+42
+xbar[5]=43
+## END
+## BUG zsh STDOUT:
+0
+xbar[5]=42
+## END
+## N-I dash status: 2
+## N-I dash stdout-json: ""
diff --git a/test/spec.sh b/test/spec.sh
index 527fd5904a..edd584cb7f 100755
--- a/test/spec.sh
+++ b/test/spec.sh
@@ -452,7 +452,7 @@ glob() {
 }
 
 arith() {
-  sh-spec spec/arith.test.sh \
+  sh-spec spec/arith.test.sh --osh-failures-allowed 4 \
     ${REF_SHELLS[@]} $ZSH $OSH_LIST "$@"
 }