Implement += on strings and arrays.

Extract EvalLhs function and share it. Still need to parse LHS indexing, e.g. a[x]+=b. Addresses issue #26.
oilshell · Aug 9, 2017 · 44b83a5595417cf180b725eb021df3f1b8b1f8a0 · 44b83a5
1 parent 0d3f679
commit 44b83a5595417cf180b725eb021df3f1b8b1f8a0
Unified Split

Showing with 105 additions and 87 deletions.

+17 −8 core/cmd_exec.py

+81 −76 core/expr_eval.py

+5 −2 spec/append.test.sh

+2 −1 test/spec.sh
diff --git a/core/cmd_exec.py b/core/cmd_exec.py
@@ -45,6 +45,7 @@
 command_e = ast.command_e
 redir_e = ast.redir_e
 lhs_expr_e = ast.lhs_expr_e
+assign_op = ast.assign_op

 value_e = runtime.value_e
 scope = runtime.scope
@@ -317,8 +318,7 @@ def _CheckStatus(self, status, node, argv0=None):
              node.__class__.__name__, status, status=status)

  def _EvalLhs(self, node):
-    """
-    """
+    """lhs_expr -> lvalue."""
    assert isinstance(node, ast.lhs_expr), node

    if node.tag == lhs_expr_e.LhsName:  # a=x
@@ -656,7 +656,7 @@ def _Dispatch(self, node, fork_external):
      elif node.keyword == Id.Assign_Readonly:
        lookup_mode = scope.Dynamic
        flags = (var_flags.ReadOnly,)
-      elif node.keyword == Id.Assign_None:
+      elif node.keyword == Id.Assign_None:  # mutate existing local or global
        lookup_mode = scope.Dynamic
        flags = ()
      else:
@@ -672,11 +672,20 @@ def _Dispatch(self, node, fork_external):
          # 'local x' is equivalent to local x=""
          val = runtime.Str('')

-        lval = self._EvalLhs(pair.lhs)
-
-        # TODO: Respect +=
-        # See expr_eval.
-        # old_val, lval = expr_eval.EvalLhs(mem, exec_opts, arith_eval)
+        if pair.op == assign_op.PlusEqual:
+          old_val, lval = expr_eval.EvalLhs(pair.lhs, self.arith_ev, self.mem,
+                                            self.exec_opts)
+          sig = (old_val.tag, val.tag)
+          if sig == (value_e.Str, value_e.Str):
+            val = runtime.Str(old_val.s + val.s)
+          elif sig == (value_e.Str, value_e.StrArray):
+            e_die("Can't append array to string")
+          elif sig == (value_e.StrArray, value_e.Str):
+            e_die("Can't append string to array")
+          elif sig == (value_e.StrArray, value_e.StrArray):
+            val = runtime.StrArray(old_val.strs + val.strs)
+        else:
+          lval = self._EvalLhs(pair.lhs)

        #log('ASSIGNING %s -> %s', lval, val)
        self.mem.SetVar(lval, val, flags, lookup_mode)
diff --git a/core/expr_eval.py b/core/expr_eval.py
@@ -101,17 +101,6 @@ def _StringToInteger(s, word=None):
  return integer


-def _ValToArith(val, word=None):
-  """Convert runtime.value to a Python int or list of strings."""
-  assert isinstance(val, runtime.value), '%r %r' % (val, type(val))
-  if val.tag == value_e.Undef:
-    return 0
-  if val.tag == value_e.Str:
-    return _StringToInteger(val.s, word=word)
-  if val.tag == value_e.StrArray:
-    return val.strs  # Python list of strings
-
-
 class _ExprEvaluator:
  """
  For now the arith and bool evaluators share some logic.
@@ -134,6 +123,83 @@ def _StringToIntegerOrError(self, s):
    return i


+def _LookupVar(name, mem, exec_opts):
+  val = mem.GetVar(name)
+  # By default, undefined variables are the ZERO value.  TODO: Respect
+  # nounset and raise an exception.
+  if val.tag == value_e.Undef and exec_opts.nounset:
+    e_die('Undefined variable %r', name)  # TODO: need token
+  return val
+
+
+def EvalLhs(node, arith_ev, mem, exec_opts):
+  """Evaluate the operand for a++ a[0]++ as an R-value.
+
+  Args:
+    node: osh_ast.lhs_expr
+
+  Returns:
+    runtime.value, runtime.lvalue
+  """
+  #log('lhs_expr NODE %s', node)
+  assert isinstance(node, ast.lhs_expr), node
+  if node.tag == lhs_expr_e.LhsName:  # a = b
+    # Problem: It can't be an array?  
+    # a=(1 2)
+    # (( a++ ))
+    lval = runtime.LhsName(node.name)
+    val = _LookupVar(node.name, mem, exec_opts)
+
+  elif node.tag == lhs_expr_e.LhsIndexedName:  # a[1] = b
+    # See tdop.IsIndexable for valid values:
+    # - ArithVarRef (not LhsName): a[1]
+    # - FuncCall: f(x), 1
+    # - ArithBinary LBracket: f[1][1] -- no semantics for this?
+
+    index = arith_ev.Eval(node.index)
+    lval = runtime.LhsIndexedName(node.name, index)
+
+    val = mem.GetVar(node.name)
+    if val.tag == value_e.Str:
+      e_die("Can't assign to characters of string %r", node.name)
+
+    elif val.tag == value_e.Undef:
+      # It would make more sense for 'nounset' to control this, but bash
+      # doesn't work that way.
+      #if self.exec_opts.strict_arith:
+      #  e_die('Undefined array %r', node.name)  # TODO: error location
+      val = runtime.Str('')
+
+    elif val.tag == value_e.StrArray:
+      #log('ARRAY %s -> %s, index %d', node.name, array, index)
+      array = val.strs
+      # NOTE: Similar logic in RHS Arith_LBracket
+      try:
+        item = array[index]
+      except IndexError:
+        val = runtime.Str('')
+      else:
+        assert isinstance(item, str), item
+        val = runtime.Str(item)
+    else:
+      raise AssertionError(val.tag)
+  else:
+    raise AssertionError(node.tag)
+
+  return val, lval
+
+
+def _ValToArith(val, word=None):
+  """Convert runtime.value to a Python int or list of strings."""
+  assert isinstance(val, runtime.value), '%r %r' % (val, type(val))
+  if val.tag == value_e.Undef:
+    return 0
+  if val.tag == value_e.Str:
+    return _StringToInteger(val.s, word=word)
+  if val.tag == value_e.StrArray:
+    return val.strs  # Python list of strings
+
+
 class ArithEvaluator(_ExprEvaluator):

  def _ValToArithOrError(self, val, word=None):
@@ -147,76 +213,15 @@ def _ValToArithOrError(self, val, word=None):
        warn(e.UserErrorString())
    return i

-  def _Lookup(self, name):
-    val = self.mem.GetVar(name)
-    # By default, undefined variables are the ZERO value.  TODO: Respect
-    # nounset and raise an exception.
-    if val.tag == value_e.Undef and self.exec_opts.nounset:
-      e_die('Undefined variable %r', name)  # TODO: need token
-    return val
-
-  def _EvalLhsToValue(self, node):
-    """Evaluate the operand for a++ a[0]++ as an R-value.
-
-    Args:
-      node: osh_ast.lhs_expr
-
-    Returns:
-      runtime.value, runtime.lvalue
-    """
-    #log('lhs_expr NODE %s', node)
-    assert isinstance(node, ast.lhs_expr), node
-    if node.tag == lhs_expr_e.LhsName:  # a = b
-      # Problem: It can't be an array?  
-      # a=(1 2)
-      # (( a++ ))
-      lval = runtime.LhsName(node.name)
-      val = self._Lookup(node.name)
-
-    elif node.tag == lhs_expr_e.LhsIndexedName:  # a[1] = b
-      # See tdop.IsIndexable for valid values:
-      # - ArithVarRef (not LhsName): a[1]
-      # - FuncCall: f(x), 1
-      # - ArithBinary LBracket: f[1][1] -- no semantics for this?
-
-      index = self.Eval(node.index)
-      lval = runtime.LhsIndexedName(node.name, index)
-
-      val = self.mem.GetVar(node.name)
-      if val.tag == value_e.Str:
-        e_die("String %r can't be assigned to", node.name)
-
-      elif val.tag == value_e.Undef:
-        # It would make more sense for 'nounset' to control this, but bash
-        # doesn't work that way.
-        #if self.exec_opts.strict_arith:
-        #  e_die('Undefined array %r', node.name)  # TODO: error location
-        val = runtime.Str('')
-
-      elif val.tag == value_e.StrArray:
-        #log('ARRAY %s -> %s, index %d', node.name, array, index)
-        array = val.strs
-        # NOTE: Similar logic in RHS Arith_LBracket
-        try:
-          item = array[index]
-        except IndexError:
-          val = runtime.Str('')
-        else:
-          assert isinstance(item, str), item
-          val = runtime.Str(item)
-      else:
-        raise AssertionError(val.tag)
-    else:
-      raise AssertionError(node.tag)
-
-    return val, lval
+  def _LookupVar(self, name):
+    return _LookupVar(name, self.mem, self.exec_opts)

  def _EvalLhsToArith(self, node):
    """
    Returns:
      int or list of strings, runtime.lvalue
    """
-    val, lval = self._EvalLhsToValue(node)
+    val, lval = EvalLhs(node, self, self.mem, self.exec_opts)
    #log('Evaluating node %r -> %r', node, val)
    return self._ValToArithOrError(val), lval

@@ -237,7 +242,7 @@ def Eval(self, node):
    # to handle that as a special case.

    if node.tag == arith_expr_e.ArithVarRef:  # $(( x ))
-      val = self._Lookup(node.name)
+      val = self._LookupVar(node.name)
      return self._ValToArithOrError(val)

    # $(( $x )) or $(( ${x}${y} )), etc.
diff --git a/spec/append.test.sh b/spec/append.test.sh
@@ -104,9 +104,12 @@ f
 # N-I mksh stdout-json: ""
 # N-I mksh status: 1

-### Append used like env prefix
-# This should be an error but it's not.
+### Append used like env prefix is a parse error
+# This should be an error in other shells but it's not.
 A=a
 A+=a printenv.py A
+# status: 2
 # BUG bash stdout: aa
+# BUG bash status: 0
 # BUG mksh stdout: a
+# BUG mksh status: 0
diff --git a/test/spec.sh b/test/spec.sh
@@ -395,7 +395,8 @@ array-compat() {

 # += is not POSIX and not in dash.
 append() {
-  sh-spec spec/append.test.sh $BASH $MKSH "$@" 
+  sh-spec spec/append.test.sh --osh-failures-allowed 4 \
+    $BASH $MKSH $OSH "$@" 
 }

 # associative array -- mksh implements different associative arrays.