Skip to content

Commit

Permalink
Bug fix: arith assignment (( i = 0 )) no longer evaluates i.
Browse files Browse the repository at this point in the history
It behaves more like i=0 in the command context.

This generated spurious warnings in bash_completion and
git-completion.bash.
  • Loading branch information
Andy Chu committed Oct 1, 2018
1 parent 4e33320 commit af99673
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 10 deletions.
44 changes: 35 additions & 9 deletions core/expr_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ def _ValToArithOrError(self, val, int_coerce=True, blame_word=None,
def _LookupVar(self, name):
return _LookupVar(name, self.mem, self.exec_opts)

def _EvalLhsToArith(self, node):
def _EvalLhsAndLookupArith(self, node):
"""
Args:
node: lhs_expr
Expand All @@ -306,6 +306,31 @@ def _EvalLhsToArith(self, node):
i = self._ValToArithOrError(val, span_id=span_id)
return i, lval

def _EvalLhsArith(self, node):
"""lhs_expr -> lvalue.
Very similar to _EvalLhs in core/cmd_exec."""
assert isinstance(node, ast.lhs_expr), node

if node.tag == lhs_expr_e.LhsName: # (( i = 42 ))
lval = runtime.LhsName(node.name)
# TODO: location info. Use the = token?
#lval.spids.append(spid)
return lval

if node.tag == lhs_expr_e.LhsIndexedName: # (( a[42] = 42 ))
# The index of StrArray needs to be coerced to int, but not the index of
# an AssocArray.
int_coerce = not self.mem.IsAssocArray(node.name, scope_e.Dynamic)
index = self.Eval(node.index, int_coerce=int_coerce)

lval = runtime.LhsIndexedName(node.name, index)
# TODO: location info. Use the = token?
#lval.spids.append(node.spids[0])
return lval

raise AssertionError(node.tag)

def _Store(self, lval, new_int):
val = runtime.Str(str(new_int))
self.mem.SetVar(lval, val, (), scope_e.Dynamic)
Expand Down Expand Up @@ -334,7 +359,7 @@ def Eval(self, node, int_coerce=True):

if node.tag == arith_expr_e.UnaryAssign: # a++
op_id = node.op_id
old_int, lval = self._EvalLhsToArith(node.child)
old_int, lval = self._EvalLhsAndLookupArith(node.child)

if op_id == Id.Node_PostDPlus: # post-increment
new_int = old_int + 1
Expand All @@ -361,16 +386,17 @@ def Eval(self, node, int_coerce=True):

if node.tag == arith_expr_e.BinaryAssign: # a=1, a+=5, a[1]+=5
op_id = node.op_id
old_int, lval = self._EvalLhsToArith(node.left)

if op_id == Id.Arith_Equal:
rhs = self.Eval(node.right)
lval = self._EvalLhsArith(node.left)
self._Store(lval, rhs)
return rhs

old_int, lval = self._EvalLhsAndLookupArith(node.left)
rhs = self.Eval(node.right)

if op_id == Id.Arith_Equal:
# NOTE: We don't need old_int for this case. Evaluating it has no side
# effects, so it's harmless.
# TODO: Use a function like core/cmd_exec.EvalLhs
new_int = rhs
elif op_id == Id.Arith_PlusEqual:
if op_id == Id.Arith_PlusEqual:
new_int = old_int + rhs
elif op_id == Id.Arith_MinusEqual:
new_int = old_int - rhs
Expand Down
8 changes: 8 additions & 0 deletions spec/bugs.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,11 @@ else
echo ELSE
fi
## stdout: ELSE

#### Turn an array into an integer.
a=(1 2 3)
(( a = 42 ))
echo $a
## stdout: 42
## N-I dash stdout-json: ""
## N-I dash status: 2
15 changes: 14 additions & 1 deletion test/runtime-errors.sh
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,19 @@ string_to_intbase() {
echo 'SHOULD NOT GET HERE'
}

undef_arith() {
(( undef++ )) # doesn't make sense

# Can't assign to characters of string? Is that strong?
(( undef[42]++ ))
}

undef_arith2() {
# undefined cell
a=()
(( a[42]++ ))
}

array_arith() {
a=(1 2)
(( a++ )) # doesn't make sense
Expand Down Expand Up @@ -357,7 +370,7 @@ all() {
failed_command \
pipefail pipefail_group pipefail_subshell pipefail_func pipefail_while \
nonexistent nounset \
nounset_arith divzero divzero_var array_arith \
nounset_arith divzero divzero_var array_arith undef_arith undef_arith2 \
string_to_int_arith string_to_hex string_to_octal \
string_to_intbase string_to_int_bool \
array_assign_1 array_assign_2 patsub_bad_glob; do
Expand Down

0 comments on commit af99673

Please sign in to comment.