Skip to content

Commit

Permalink
[spec/arith] Failing cases for dynamic LHS name in arith assignment.
Browse files Browse the repository at this point in the history
Addresses issue #640.
  • Loading branch information
Andy Chu committed Mar 8, 2020
1 parent 583a925 commit 8fdb25d
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 11 deletions.
2 changes: 2 additions & 0 deletions frontend/syntax.asdl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion osh/arith_parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand All @@ -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)

Expand Down
16 changes: 8 additions & 8 deletions osh/sh_expr_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
#


Expand Down Expand Up @@ -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

Expand Down
6 changes: 5 additions & 1 deletion osh/tdop.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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):
Expand All @@ -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
Expand Down
52 changes: 52 additions & 0 deletions spec/arith.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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: ""
2 changes: 1 addition & 1 deletion test/spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 "$@"
}

Expand Down

0 comments on commit 8fdb25d

Please sign in to comment.