[oil-language] command_sub_errexit always set during Oil expr eval

In other words, this is FATAL even in bin/osh.

    var x = $(false)

You must use the 'try' builtin to recover from it.

The 'var' keyword is a new construct that doesn't require compatibility,
so we it should have the right behavior in bin/osh.

This finishes #942.

core/shell.py

@@ -444,7 +444,7 @@ def Main(lang, arg_r, environ, login_shell, loader, line_input):
 
   arith_ev = sh_expr_eval.ArithEvaluator(mem, exec_opts, parse_ctx, errfmt)
   bool_ev = sh_expr_eval.BoolEvaluator(mem, exec_opts, parse_ctx, errfmt)
-  expr_ev = expr_eval.OilEvaluator(mem, procs, splitter, errfmt)
+  expr_ev = expr_eval.OilEvaluator(mem, mutable_opts, procs, splitter, errfmt)
   word_ev = word_eval.NormalWordEvaluator(mem, exec_opts, mutable_opts,
                                           splitter, errfmt)
 

core/state.py

@@ -200,6 +200,22 @@ def __exit__(self, type, value, traceback):
       self.mutable_opts.Pop(option_i.allow_csub_psub)
 
 
+class ctx_OilExpr(object):
+  """ Command sub must fail in 'mystring' ++ $(false) """
+  def __init__(self, mutable_opts):
+    # type: (MutableOpts) -> None
+    mutable_opts.Push(option_i.command_sub_errexit, True)
+    self.mutable_opts = mutable_opts
+
+  def __enter__(self):
+    # type: () -> None
+    pass
+
+  def __exit__(self, type, value, traceback):
+    # type: (Any, Any, Any) -> None
+    self.mutable_opts.Pop(option_i.command_sub_errexit)
+
+
 class ctx_ErrExit(object):
   """Manages the errexit setting.
 

oil_lang/expr_eval.py

@@ -14,6 +14,7 @@
 )
 from asdl import runtime
 from core import error
+from core import state
 from core.pyerror import e_die, log
 from frontend import consts
 from oil_lang import objects
@@ -68,6 +69,7 @@ class OilEvaluator(object):
 
   def __init__(self,
                mem,  # type: Mem
+               mutable_opts,  # type: state.MutableOpts
                funcs,  # type: Dict
                splitter,  # type: split.SplitContext
                errfmt,  # type: ErrorFormatter
@@ -77,8 +79,9 @@ def __init__(self,
     self.word_ev = None  # type: StringWordEvaluator
 
     self.mem = mem
-    self.splitter = splitter
+    self.mutable_opts = mutable_opts
     self.funcs = funcs
+    self.splitter = splitter
     self.errfmt = errfmt
 
   def CheckCircularDeps(self):
@@ -187,6 +190,12 @@ def EvalPlaceExpr(self, place):
     raise NotImplementedError(place)
 
   def EvalExpr(self, node):
+    # type: (expr_t) -> Any
+    """Public API for _EvalExpr that ensures that command_sub_errexit is on."""
+    with state.ctx_OilExpr(self.mutable_opts):
+      return self._EvalExpr(node)
+
+  def _EvalExpr(self, node):
     # type: (expr_t) -> Any
     """
     This is a naive PyObject evaluator!  It uses the type dispatch of the host
@@ -197,7 +206,7 @@ def EvalExpr(self, node):
       storing in Mem.
     """
     if 0:
-      print('EvalExpr()')
+      print('_EvalExpr()')
       node.PrettyPrint()
       print('')
 
@@ -291,7 +300,7 @@ def EvalExpr(self, node):
       return self.word_ev.EvalSimpleVarSubToString(node.token)
 
     if node.tag == expr_e.Unary:
-      child = self.EvalExpr(node.child)
+      child = self._EvalExpr(node.child)
       if node.op.id == Id.Arith_Minus:
         return -child
       if node.op.id == Id.Arith_Tilde:
@@ -302,8 +311,8 @@ def EvalExpr(self, node):
       raise NotImplementedError(node.op.id)
 
     if node.tag == expr_e.Binary:
-      left = self.EvalExpr(node.left)
-      right = self.EvalExpr(node.right)
+      left = self._EvalExpr(node.left)
+      right = self._EvalExpr(node.right)
 
       if node.op.id == Id.Arith_Plus:
         return left + right
@@ -353,21 +362,21 @@ def EvalExpr(self, node):
       raise NotImplementedError(node.op.id)
 
     if node.tag == expr_e.Range:  # 1:10  or  1:10:2
-      lower = self.EvalExpr(node.lower)
-      upper = self.EvalExpr(node.upper)
+      lower = self._EvalExpr(node.lower)
+      upper = self._EvalExpr(node.upper)
       return xrange(lower, upper)
 
     if node.tag == expr_e.Slice:  # a[:0]
-      lower = self.EvalExpr(node.lower) if node.lower else None
-      upper = self.EvalExpr(node.upper) if node.upper else None
+      lower = self._EvalExpr(node.lower) if node.lower else None
+      upper = self._EvalExpr(node.upper) if node.upper else None
       return slice(lower, upper)
 
     if node.tag == expr_e.Compare:
-      left = self.EvalExpr(node.left)
+      left = self._EvalExpr(node.left)
       result = True  # Implicit and
       for op, right_expr in zip(node.ops, node.comparators):
 
-        right = self.EvalExpr(right_expr)
+        right = self._EvalExpr(right_expr)
 
         if op.id == Id.Arith_Less:
           result = left < right
@@ -450,28 +459,28 @@ def EvalExpr(self, node):
       return result
 
     if node.tag == expr_e.IfExp:
-      b = self.EvalExpr(node.test)
+      b = self._EvalExpr(node.test)
       if b:
-        return self.EvalExpr(node.body)
+        return self._EvalExpr(node.body)
       else:
-        return self.EvalExpr(node.orelse)
+        return self._EvalExpr(node.orelse)
 
     if node.tag == expr_e.List:
-      return [self.EvalExpr(e) for e in node.elts]
+      return [self._EvalExpr(e) for e in node.elts]
 
     if node.tag == expr_e.Tuple:
-      return tuple(self.EvalExpr(e) for e in node.elts)
+      return tuple(self._EvalExpr(e) for e in node.elts)
 
     if node.tag == expr_e.Dict:
       # NOTE: some keys are expr.Const
-      keys = [self.EvalExpr(e) for e in node.keys]
+      keys = [self._EvalExpr(e) for e in node.keys]
 
       values = []
       for i, e in enumerate(node.values):
         if e.tag == expr_e.Implicit:
           v = self.LookupVar(keys[i])  # {name}
         else:
-          v = self.EvalExpr(e)
+          v = self._EvalExpr(e)
         values.append(v)
 
       return dict(zip(keys, values))
@@ -484,7 +493,7 @@ def EvalExpr(self, node):
       #   Hm... lexical or dynamic scope is an issue.
       result = []
       comp = node.generators[0]
-      obj = self.EvalExpr(comp.iter)
+      obj = self._EvalExpr(comp.iter)
 
       # TODO: Handle x,y etc.
       iter_name = comp.lhs[0].name.val
@@ -503,19 +512,19 @@ def EvalExpr(self, node):
             lvalue.Named(iter_name), value.Obj(loop_val), scope_e.LocalOnly)
 
         if comp.cond:
-          b = self.EvalExpr(comp.cond)
+          b = self._EvalExpr(comp.cond)
         else:
           b = True
 
         if b:
-          item = self.EvalExpr(node.elt)  # e.g. x*2
+          item = self._EvalExpr(node.elt)  # e.g. x*2
           result.append(item)
 
       return result
 
     if node.tag == expr_e.GeneratorExp:
       comp = node.generators[0]
-      obj = self.EvalExpr(comp.iter)
+      obj = self._EvalExpr(comp.iter)
 
       # TODO: Support (x for x, y in ...)
       iter_name = comp.lhs[0].name.val
@@ -535,12 +544,12 @@ def _gen():
               lvalue.Named(iter_name), value.Obj(loop_val), scope_e.LocalOnly)
 
           if comp.cond:
-            b = self.EvalExpr(comp.cond)
+            b = self._EvalExpr(comp.cond)
           else:
             b = True
 
           if b:
-            item = self.EvalExpr(node.elt)  # e.g. x*2
+            item = self._EvalExpr(node.elt)  # e.g. x*2
             yield item
 
       return _gen()
@@ -551,13 +560,13 @@ def _gen():
       #return objects.Lambda(node, None)
 
     if node.tag == expr_e.FuncCall:
-      func = self.EvalExpr(node.func)
+      func = self._EvalExpr(node.func)
       pos_args, named_args = self.EvalArgList(node.args)
       ret = func(*pos_args, **named_args)
       return ret
 
     if node.tag == expr_e.Subscript:
-      obj = self.EvalExpr(node.obj)
+      obj = self._EvalExpr(node.obj)
       index = self._EvalIndices(node.indices)
       try:
         result = obj[index]
@@ -573,7 +582,7 @@ def _gen():
     # Note: This is only for the obj.method() case.  We will probably change
     # the AST and get rid of getattr().
     if node.tag == expr_e.Attribute:  # obj.attr 
-      o = self.EvalExpr(node.obj)
+      o = self._EvalExpr(node.obj)
       id_ = node.op.id
       if id_ == Id.Expr_Dot:
         # Used for .startswith()
@@ -605,7 +614,7 @@ def _gen():
       return objects.Regex(self.EvalRegex(node.regex))
 
     if node.tag == expr_e.ArrayLiteral:  # obj.attr 
-      items = [self.EvalExpr(item) for item in node.items]
+      items = [self._EvalExpr(item) for item in node.items]
       if items:
         # Determine type at runtime?  If we have something like @[(i) (j)]
         # then we don't know its type until runtime.

spec/oil-assign.test.sh

@@ -314,19 +314,38 @@ setvar L[0] = L
 # NOTE: This feels PROBLEMATIC without command_sub_errexit feels like it should
 # be the last one ...
 
+$SH -c '
 var x = $(false)
-echo x=$x status=$?
-
-setvar x = "$(false)/$(echo 42; exit 42)"
-echo x=$x status=$?
-
-const y = "$(false)/$(echo 43; exit 43)/$(echo 44; exit 44)"
-echo y=$y status=$?
+echo inside=$?
+'
+echo outside=$?
+
+$SH -c '
+setvar x = $(false)
+echo inside=$?
+'
+echo outside=$?
+
+# Argument list
+$SH -c '
+_ split( $(false) )
+echo inside=$?
+'
+echo outside=$?
+
+# Place expression
+$SH -c '
+var d = {}
+setvar d[ $(false) ] = 42
+echo inside=$?
+'
+echo outside=$?
 
 ## STDOUT:
-x= status=1
-x=/42 status=1
-y=/43/44 status=1
+outside=1
+outside=1
+outside=1
+outside=1
 ## END
 
 

spec/oil-builtin-error.test.sh

@@ -280,6 +280,24 @@ hi
 _status=3
 ## END
 
+#### try with failed command sub within expression 
+shopt --set parse_brace
+
+try {
+  echo hi
+  var x = $(exit 42)  # errexit
+  echo bye
+}
+echo try $_status
+
+# Note that there's no way to retrieve this status WITHOUT try
+# var x = $(exit 42)  # errexit
+
+## STDOUT:
+hi
+try 42
+## END
+
 #### Uncaught expression error exits status 3
 $SH -c '
 

test/spec.sh

@@ -884,7 +884,7 @@ oil-array() {
 }
 
 oil-assign() {
-  sh-spec spec/oil-assign.test.sh --osh-failures-allowed 1 \
+  sh-spec spec/oil-assign.test.sh --osh-failures-allowed 0 \
     $OSH_LIST "$@"
 }