Fix bug in case execution tickled by OCaml's configure script.

If two alternatives in a case clause matched the string, the action would get executed twice. Also: - Started a mechanism to trace the source locations where a variable is set. This helped me track down this problem. - benchmarks/runtime.sh: Copy the output of the files the configure scripts touched so we can compare them. This helped narrow down more than one bug.
oilshell · Dec 2, 2017 · 143b77aa0be14cb404258704663b1f07a942e2f9 · 143b77a
1 parent 6245833
commit 143b77aa0be14cb404258704663b1f07a942e2f9
Unified Split

Showing with 77 additions and 17 deletions.

+39 −12 benchmarks/runtime.sh

+1 −1 bin/oil.py

+8 −3 core/cmd_exec.py

+20 −1 core/state.py

+9 −0 spec/case_.test.sh
diff --git a/benchmarks/runtime.sh b/benchmarks/runtime.sh
@@ -1,7 +1,13 @@
 #!/bin/bash
 #
+# Test scripts found in the wild for both correctness and performance.
+#
 # Usage:
 #   ./runtime.sh <function name>
+#
+# TODO:
+# - Merge this with test/wild2.sh?  benchmarks/wild3.sh or test/wild3.sh?
+# - abuild -h -- time it

 set -o nounset
 set -o pipefail
@@ -17,7 +23,7 @@ uftrace-0.8.1.tar.gz
 EOF
 }

-readonly TAR_DIR=_tmp/benchmarks/runtime 
+readonly TAR_DIR=$PWD/_tmp/benchmarks/runtime 
 readonly OSH=$PWD/bin/osh

 download() {
@@ -32,41 +38,62 @@ extract() {
  ls -l $TAR_DIR
 }

-configure-and-show-new() {
-  local dir=$1
+configure-and-copy() {
+  local src_dir=$1
+  local sh=$2
+  local out_dir=$3
+
+  mkdir -p $out_dir
+
+  # These hand-written configure scripts must be run from their own directory,
+  # unlike autoconf's scripts.

-  pushd $dir >/dev/null
+  pushd $src_dir >/dev/null
  touch __TIMESTAMP
  #$OSH -x ./configure
-  $OSH ./configure
+  $sh ./configure

  echo
  echo "--- NEW FILES ---"
  echo

-  find . -type f -newer __TIMESTAMP
+  find . -type f -newer __TIMESTAMP | xargs -I {} --verbose -- cp {} $out_dir
  popd >/dev/null
 }

+configure-twice() {
+  local dir=$1
+  local label=$(basename $dir)
+  configure-and-copy $dir bash $TAR_DIR/${label}__bash
+  configure-and-copy $dir dash $TAR_DIR/${label}__dash
+  configure-and-copy $dir $OSH $TAR_DIR/${label}__osh
+}
+
+
 # TODO: Run under bash and osh.  Look for all the files that changed?  Using
 # 'find'?  And then diff them.

 yash() {
-  configure-and-show-new $TAR_DIR/yash-2.46
+  configure-twice $TAR_DIR/yash-2.46
 }

-# test expression problem
+# Works for bash/dash/osh!
 tcc() {
-  configure-and-show-new $TAR_DIR/tcc-0.9.26
+  configure-twice $TAR_DIR/tcc-0.9.26
+  #configure-and-show-new $TAR_DIR/tcc-0.9.26
 }

-# What is the s:?
+# Works for bash/dash/osh!
 uftrace() {
-  configure-and-show-new $TAR_DIR/uftrace-0.8.1
+  configure-twice $TAR_DIR/uftrace-0.8.1
+  #configure-and-show-new $TAR_DIR/uftrace-0.8.1
 }

+# Flags are different.
 ocaml() {
-  configure-and-show-new $TAR_DIR/ocaml-4.06.0
+  configure-twice $TAR_DIR/ocaml-4.06.0
+  #mkdir -p _tmp/ocaml
+  #configure-and-copy $TAR_DIR/ocaml-4.06.0 $OSH $PWD/_tmp/ocaml
 }

 # Same problem as tcc
diff --git a/bin/oil.py b/bin/oil.py
@@ -238,7 +238,7 @@ def OshMain(argv, login_shell):
  # TODO: Maybe wrap this initialization sequence up in an oil_State, like
  # lua_State.
  status_lines = ui.MakeStatusLines()
-  mem = state.Mem(dollar0, argv[opt_index + 1:], os.environ)
+  mem = state.Mem(dollar0, argv[opt_index + 1:], os.environ, arena)
  funcs = {}

  # Passed to Executor for 'complete', and passed to completion.Init
diff --git a/core/cmd_exec.py b/core/cmd_exec.py
@@ -348,12 +348,14 @@ def _CheckStatus(self, status, node, argv0=None):
        e_die('[%d] %r exited with status %d', os.getpid(),
              node.__class__.__name__, status, status=status)

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

    if node.tag == lhs_expr_e.LhsName:  # a=x
-      return runtime.LhsName(node.name)
+      node = runtime.LhsName(node.name)
+      node.spids.append(spid)
+      return node

    if node.tag == lhs_expr_e.LhsIndexedName:  # a[1+2]=x
      i = self.arith_ev.Eval(node.index)
@@ -461,6 +463,7 @@ def _EvalEnv(self, node_env, out_env):

      # Set each var so the next one can reference it.  Example:
      # FOO=1 BAR=$FOO ls /
+      # TODO: Could add spid to LhsName.
      self.mem.SetVar(ast.LhsName(name), val, (), scope_e.LocalOnly)

      out_env[name] = val.s
@@ -722,7 +725,8 @@ def _Dispatch(self, node, fork_external):
            val = runtime.StrArray(old_val.strs + val.strs)

        else:  # plain assignment
-          lval = self._EvalLhs(pair.lhs)
+          spid = pair.spids[0]  # Source location for tracing
+          lval = self._EvalLhs(pair.lhs, spid)

          # RHS can be a string or array.
          if pair.rhs:
@@ -885,6 +889,7 @@ def _Dispatch(self, node, fork_external):
          if libc.fnmatch(pat_val.s, to_match):
            status = self._ExecuteList(arm.action)
            done = True  # TODO: Parse ;;& and for fallthrough and such?
+            break  # Only execute action ONCE
        if done:
          break

diff --git a/core/state.py b/core/state.py
@@ -206,7 +206,7 @@ class Mem(object):
  Modules: cmd_exec, word_eval, expr_eval, completion
  """

-  def __init__(self, argv0, argv, environ):
+  def __init__(self, argv0, argv, environ, arena):
    top = {}  # string -> runtime.cell
    self.var_stack = [top]
    self.argv0 = argv0
@@ -224,6 +224,7 @@ def __init__(self, argv0, argv, environ):

    self._InitDefaults()
    self._InitEnviron(environ)
+    self.arena = arena

  def __repr__(self):
    parts = []
@@ -393,6 +394,24 @@ def SetVar(self, lval, value, new_flags, lookup_mode):
    assert new_flags is not None

    if lval.tag == lvalue_e.LhsName:
+      #if lval.name == 'ldflags':
+      # TODO: Turn this into a tracing feature.  Like osh --tracevar ldflags
+      # --tracevar foo.  Has to respect environment variables too.
+      if 0:
+        util.log('--- SETTING ldflags to %s', value)
+        if lval.spids:
+          span_id = lval.spids[0]
+          line_span = self.arena.GetLineSpan(span_id)
+          line_id = line_span.line_id
+          #line = arena.GetLine(line_id)
+          path, line_num = self.arena.GetDebugInfo(line_id)
+          col = line_span.col
+          #length = line_span.length
+          util.log('--- spid %s: %s, line %d, col %d', span_id, path,
+                   line_num+1, col)
+
+          # TODO: Need the arena to look it up the line spid and line number.
+
      # Maybe this should return one of (cell, scope).  existing cell, or the
      # scope to put it in?
      # _FindCellOrScope
diff --git a/spec/case_.test.sh b/spec/case_.test.sh
@@ -63,3 +63,12 @@ case "$x" in
  "$pat") echo match ;;
 esac
 # stdout: match
+
+### Multiple Patterns Match
+x=foo
+result='-'
+case "$x" in
+  f*|*o) result="$result X"
+esac
+echo $result
+# stdout: - X