Make the behavior of top-level return/break/continue follow dash/mksh.

Bash's behavior is inconsistent.  If you can return from a source'd
script, you should be able to return from the main script.

This behavior is OK for running Aboriginal Linux.  But the old behavior
of disallowing return at the top level was not OK, and it didn't match
that of any shell.

Also add set -o strict-control-flow -- the first STRICT option.  It
disallows 'continue' and 'break' at the top level, but allows 'return'.

README.md

@@ -72,6 +72,9 @@ Directory Structure
      smoke.sh
      sh_spec.py      # shell spec test framework
    spec/             # spec test cases
      bin/            # tools used in many spec tests
      testdata/       # scripts for specific test cases
      errors/         # TODO: migrate these bad shell scripts
    scripts/          # Other development scripts
    web/              # HTML/JS/CSS for tests and tools

core/args.py

@@ -72,6 +72,8 @@ def __init__(self, defaults):
    for name, v in defaults.iteritems():
      setattr(self, name, v)

  # TODO: Instead of setattr(), use Set(name, val), and change '-' to '_'?

  def __repr__(self):
    return '<_Attributes %s>' % self.__dict__

core/builtin.py

@@ -614,7 +614,7 @@ def AddOptionsToArgSpec(spec):
def SetExecOpts(exec_opts, opt_changes):
  """Used by bin/oil.py too."""
  for opt_name, b in opt_changes:
    exec_opts.SetOption(opt_name, b)
    exec_opts.SetOption(opt_name.replace('-', '_'), b)


def Set(argv, exec_opts, mem):
@@ -631,7 +631,6 @@ def Set(argv, exec_opts, mem):

  arg, i = SET_SPEC.Parse(argv)

  # TODO: exec_opts.SetOption()
  SetExecOpts(exec_opts, arg.opt_changes)
  if arg.saw_double_dash or i != len(argv):  # set -u shouldn't affect argv
    mem.SetArgv(argv[i:])

core/cmd_exec.py

@@ -88,6 +88,9 @@ def ReturnValue(self):
    assert self.IsReturn()
    return self.arg

  def __repr__(self):
    return '<_ControlFlow %s>' % self.token


class Executor(object):
  """Executes the program by tree-walking.
@@ -136,6 +139,8 @@ def __init__(self, mem, status_lines, funcs, completion, comp_lookup,
    # sleep 5 & puts a (PID, job#) entry here.  And then "jobs" displays it.
    self.job_state = process.JobState()

    self.loop_level = 0  # for detecting bad top-level break/continue

  def _Complete(self, argv):
    """complete builtin - register a completion function.

@@ -174,8 +179,10 @@ def _EvalHelper(self, c_parser, source_name):
        err = c_parser.Error()
        ui.PrintErrorStack(err, self.arena, sys.stderr)
        return 1

      status = self._Execute(node)
      return status

    finally:
      self.arena.PopSource()

@@ -194,10 +201,12 @@ def _Source(self, argv):
      # TODO: Should point to the source statement that failed.
      util.error('source: missing required argument')
      return 1

    try:
      with open(path) as f:
        line_reader = reader.FileLineReader(f, self.arena)
        _, c_parser = parse_lib.MakeParser(line_reader, self.arena)
        # TODO: We have to set a bit to know that we are in 'source'
        return self._EvalHelper(c_parser, path)
    except _ControlFlow as e:
      if e.IsReturn():
@@ -749,8 +758,27 @@ def _Dispatch(self, node, fork_external):
      else:
        arg = 0  # return 0, break 0 levels, etc.

      # NOTE: always raises so we don't set status.
      raise _ControlFlow(node.token, arg)
      # NOTE: We don't do anything about a top-level 'return' here.  Unlike in
      # bash, that is OK.  If you can return from a sourced script, it makes
      # sense to return from a main script.
      ok = True
      tok = node.token
      if (tok.id in (Id.ControlFlow_Break, Id.ControlFlow_Continue) and
          self.loop_level == 0):
        ok = False
        msg = 'Invalid control flow at top level'
      
      if ok:
        raise _ControlFlow(tok, arg)

      if self.exec_opts.strict_control_flow:
        e_die(msg, token=tok)
      else:
        # Only print warnings, never fatal.
        # Bash oddly only exits 1 for 'return', but no other shell does.
        ui.PrintFilenameAndLine(tok.span_id, self.arena)
        util.warn(msg)
        status = 0

    # The only difference between these two is that CommandList has no
    # redirects.  We already took care of that above.
@@ -791,27 +819,32 @@ def _Dispatch(self, node, fork_external):
        _DonePredicate = lambda status: status == 0

      status = 0
      while True:
        self._PushErrExit()
        try:
          cond_status = self._ExecuteList(node.cond)
        finally:
          self._PopErrExit()

        done = cond_status != 0
        if _DonePredicate(cond_status):
          break
        try:
          status = self._Execute(node.body)  # last one wins
        except _ControlFlow as e:
          if e.IsBreak():
            status = 0
      self.loop_level += 1
      try:
        while True:
          self._PushErrExit()
          try:
            cond_status = self._ExecuteList(node.cond)
          finally:
            self._PopErrExit()

          done = cond_status != 0
          if _DonePredicate(cond_status):
            break
          elif e.IsContinue():
            status = 0
            continue
          else:  # return needs to pop up more
            raise
          try:
            status = self._Execute(node.body)  # last one wins
          except _ControlFlow as e:
            if e.IsBreak():
              status = 0
              break
            elif e.IsContinue():
              status = 0
              continue
            else:  # return needs to pop up more
              raise
      finally:
        self.loop_level -= 1

    elif node.tag == command_e.ForEach:
      iter_name = node.iter_name
@@ -823,23 +856,28 @@ def _Dispatch(self, node, fork_external):
        # We need word splitting and so forth
        # NOTE: This expands globs too.  TODO: We should pass in a Globber()
        # object.
      status = 0  # in case we don't loop
      for x in iter_list:
        #log('> ForEach setting %r', x)
        state.SetLocalString(self.mem, iter_name, x)
        #log('<')

        try:
          status = self._Execute(node.body)  # last one wins
        except _ControlFlow as e:
          if e.IsBreak():
            status = 0
            break
          elif e.IsContinue():
            status = 0
            continue
          else:  # return needs to pop up more
            raise
      status = 0  # in case we don't loop
      self.loop_level += 1
      try:
        for x in iter_list:
          #log('> ForEach setting %r', x)
          state.SetLocalString(self.mem, iter_name, x)
          #log('<')

          try:
            status = self._Execute(node.body)  # last one wins
          except _ControlFlow as e:
            if e.IsBreak():
              status = 0
              break
            elif e.IsContinue():
              status = 0
              continue
            else:  # return needs to pop up more
              raise
      finally:
        self.loop_level -= 1

    elif node.tag == command_e.ForExpr:
      raise NotImplementedError(node.tag)
@@ -984,10 +1022,11 @@ def Execute(self, node, fork_external=True):
    try:
      status = self._Execute(node, fork_external=fork_external)
    except _ControlFlow as e:
      # NOTE: in bash return is a warning.  Maybe have a sane-* option?
      ui.PrintFilenameAndLine(e.token.span_id, self.arena)
      log('osh failed: Unexpected %r at top level' % e.token.val)
      status = 1
      # Return at top level is OK, unlike in bash.
      if e.IsReturn():
        status = e.ReturnValue()
      else:
        raise
    except util.FatalRuntimeError as e:
      ui.PrettyPrintError(e, self.arena)
      print('osh failed: %s' % e.UserErrorString(), file=sys.stderr)

core/state.py

@@ -74,9 +74,14 @@ def Set(self, b):
    (None, 'pipefail'),

    (None, 'debug-completion'),

    (None, 'strict-control-flow'),
]

_SET_OPTION_NAMES = set(name for _, name in SET_OPTIONS)
# NOTE: We have to change - to _ here, because we need to match _Attributes in
# core/args.py.

_SET_OPTION_NAMES = set(name.replace('-', '_') for _, name in SET_OPTIONS)


class ExecOpts(object):
@@ -91,6 +96,7 @@ def __init__(self):
    self.noexec = False  # -n
    self.noclobber = False  # -C
    self.debug_completion = False
    self.strict_control_flow = False

    # shopt -s / -u
    self.nullglob = False 
@@ -131,11 +137,14 @@ def GetDollarHyphen(self):

  def SetOption(self, opt_name, b):
    """ For set -o, set +o, or shopt -s/-u -o. """
    assert '-' not in opt_name, 'Option names should have _, not -'
    if opt_name not in _SET_OPTION_NAMES:
      raise args.UsageError('Invalid option %r' % opt_name)
    if opt_name == 'errexit':
      self.errexit.Set(b)
    else:
      # strict-control-flow -> strict_control_flow
      opt_name = opt_name.replace('-', '_')
      setattr(self, opt_name, b)

  SHOPT_OPTIONS = ('nullglob', 'failglob')

spec/builtins.test.sh

@@ -95,13 +95,6 @@ echo status=$?
# OK bash stdout: status=2
# OK dash stdout: status=127

### Source script that returns
echo one
. spec/return-helper.sh
echo $?
echo two
# stdout-json: "one\nreturn-helper.sh\n42\ntwo\n"

### Exit builtin
exit 3
# status: 3

spec/func.test.sh

@@ -26,15 +26,6 @@ f
# stdout: one
# status: 42

### Return at top level is error
return
echo bad
# N-I dash/mksh status: 0
# N-I bash status: 0
# N-I bash stdout: bad
# status: 1
# stdout-json: ""

### Dynamic Scope
f() {
  echo $g_var

spec/loop.test.sh

@@ -72,23 +72,6 @@ done
# status: 0
# stdout-json: "a\nb\n"

### continue at top level is error
# NOTE: bash and mksh both print warnings, but don't exit with an error.
continue
echo bad
# N-I bash/dash/mksh status: 0
# N-I bash/dash/mksh stdout: bad
# stdout-json: ""
# status: 1

### break at top level is error
break
echo bad
# N-I bash/dash/mksh status: 0
# N-I bash/dash/mksh stdout: bad
# stdout-json: ""
# status: 1

### while in while condition
# This is a consequence of the grammar
while while true; do echo cond; break; done

spec/osh-only.test.sh

@@ -3,3 +3,11 @@
### debug-line builtin
debug-line 'hi there'
# status: 0

### debug-completion option
set -o debug-completion
# status: 0

### debug-completion from command line
$SH -o debug-completion
# status: 0

spec/strict-options.test.sh

@@ -0,0 +1,46 @@
#!/usr/bin/env bash

### Sourcing a script that returns is allowed no matter what
echo one
. spec/testdata/return-helper.sh
echo $?
echo two
# stdout-json: "one\nreturn-helper.sh\n42\ntwo\n"

### top level control flow
$SH spec/testdata/top-level-control-flow.sh
# stdout-json: "SUBSHELL\nBREAK\nCONTINUE\nRETURN\n"
# OK bash stdout-json: "SUBSHELL\nBREAK\nCONTINUE\nRETURN\nDONE\n"
# status: 0

### errexit and top-level control flow
$SH -o errexit spec/testdata/top-level-control-flow.sh
# stdout-json: "SUBSHELL\n"
# status: 2
# OK bash status: 1

### set -o strict-control-flow
$SH -o strict-control-flow -c 'echo break; break; echo hi'
# stdout: break
# status: 1
# N-I dash/bash status: 2
# N-I dash/bash stdout-json: ""
# N-I mksh status: 1
# N-I mksh stdout-json: ""

### return at top level is an error
return
echo "status=$?"
# stdout-json: ""
# OK bash stdout-json: "status=1\n"

### continue at top level is NOT an error
# NOTE: bash and mksh both print warnings, but don't exit with an error.
continue
echo status=$?
# stdout: status=0

### break at top level is NOT an error
break
echo status=$?
# stdout: status=0