Fix bug parsing test expression (found in tcc's and QEMU's configure)

The expression is of the form: 'test -z x -o z = z' It only happened in expression longer than 4 tokens to trigger the osh/bool_parse.py rather than the heuristics. The bug was that (BoolBinary_Equal '=') was assigned the wrong kind, because it is setup after all the other id / kind pairs. Added a unit test. All spec tests pass. Also: - Polish the error message when there is a bad 'test' expression - Show the files that a configure script changed in benchmarks/runtime.sh.
oilshell · Dec 2, 2017 · 8402bd68a562b21e14e3fc0c701bf09c73d157d1 · 8402bd6
1 parent 1ea42c1
commit 8402bd68a562b21e14e3fc0c701bf09c73d157d1
Unified Split

Showing with 73 additions and 17 deletions.

+36 −4 benchmarks/runtime.sh

+16 −5 core/id_kind.py

+3 −0 core/id_kind_test.py

+4 −2 core/test_builtin.py

+7 −2 core/util.py

+7 −4 osh/bool_parse.py
diff --git a/benchmarks/runtime.sh b/benchmarks/runtime.sh
@@ -26,19 +26,51 @@ download() {
 }

 extract() {
-  time for f in $TAR_DIR/*z; do
+  time for f in $TAR_DIR/*.{xz,bz2}; do
    tar -x --directory $TAR_DIR --file $f 
  done
  ls -l $TAR_DIR
 }

+configure-and-show-new() {
+  local dir=$1
+
+  pushd $dir >/dev/null
+  touch __TIMESTAMP
+  $OSH ./configure
+
+  echo
+  echo "--- NEW FILES ---"
+  echo
+
+  find . -type f -newer __TIMESTAMP
+  popd >/dev/null
+}
+
 # TODO: Run under bash and osh.  Look for all the files that changed?  Using
 # 'find'?  And then diff them.

 yash() {
-  pushd $TAR_DIR/yash-2.46
-  $OSH ./configure
-  popd
+  configure-and-show-new $TAR_DIR/yash-2.46
+}
+
+# test expression problem
+tcc() {
+  configure-and-show-new $TAR_DIR/tcc-0.9.26
+}
+
+# What is the s:?
+uftrace() {
+  configure-and-show-new $TAR_DIR/uftrace-0.8.1
+}
+
+ocaml() {
+  configure-and-show-new $TAR_DIR/ocaml-4.06.0
+}
+
+# Same problem as tcc
+qemu-old() {
+  configure-and-show-new ~/src/qemu-1.6.0
 }

 "$@"
diff --git a/core/id_kind.py b/core/id_kind.py
@@ -59,6 +59,9 @@ def __repr__(self):

 class Kind(object):
  """A coarser version of Id, used to make parsing decisions."""
+
+  # TODO: The Kind type should be folded into ASDL.  It can't print itself,
+  # which is inconsistent with Id.
  pass


@@ -84,19 +87,27 @@ def __init__(self, token_names, instance_lookup, kind_lookup, bool_ops):
  def LexerPairs(self, kind):
    return self.lexer_pairs[kind]

-  def _AddId(self, token_name):
+  def _AddId(self, token_name, kind=None):
+    """
+    Args:
+      token_name: e.g. BoolBinary_Equal
+      kind: override autoassignment.  For AddBoolBinaryForBuiltin
+    """
    self.token_index += 1  # leave out 0 I guess?
    id_val = Id(self.token_index)
    setattr(self.id_enum, token_name, id_val)

    t = self.token_index
    self.token_names[t] = token_name
    self.instance_lookup[t] = id_val
-    self.kind_lookup[t] = self.kind_index
+    if kind is None:
+      kind = self.kind_index
+    self.kind_lookup[t] = kind
    return id_val

  def _AddKind(self, kind_name):
    setattr(self.kind_enum, kind_name, self.kind_index)
+    #util.log('%s = %d', kind_name, self.kind_index)
    self.kind_index += 1

  def AddKind(self, kind_name, tokens):
@@ -152,13 +163,13 @@ def AddBoolKind(self, kind_name, arg_type_pairs):
    self._AddKind(kind_name)
    self.kind_sizes.append(num_tokens)  # debug info

-  def AddBoolBinaryForBuiltin(self, token_name):
+  def AddBoolBinaryForBuiltin(self, token_name, kind):
    """For [ = ] [ == ] and [ != ].
    
    These operators are NOT added to the lexer.  The are "lexed" as StringWord.
    """
    token_name = 'BoolBinary_%s' % token_name
-    id_val = self._AddId(token_name)
+    id_val = self._AddId(token_name, kind=kind)
    self.AddBoolOp(id_val, OperandType.Str)
    return id_val

@@ -457,7 +468,7 @@ def _SetupTestBuiltin(id_spec, unary_lookup, binary_lookup, other_lookup):

  for token_name, token_str in [
      ('Equal', '='), ('DEqual', '=='), ('NEqual', '!=')]:
-    id_val = id_spec.AddBoolBinaryForBuiltin(token_name)
+    id_val = id_spec.AddBoolBinaryForBuiltin(token_name, Kind.BoolBinary)
    binary_lookup[token_str] = id_val

  # Some of these names don't quite match, but it keeps the BoolParser simple.
diff --git a/core/id_kind_test.py b/core/id_kind_test.py
@@ -57,6 +57,9 @@ def testTokens(self):
    t = ast.token(Id.BoolBinary_GlobDEqual, '==')
    self.assertEqual(Kind.BoolBinary, LookupKind(t.id))

+    t = ast.token(Id.BoolBinary_Equal, '=')
+    self.assertEqual(Kind.BoolBinary, LookupKind(t.id))
+
  def testEquality(self):
    # OK WTF!!!!
    left = Id(198)
diff --git a/core/test_builtin.py b/core/test_builtin.py
@@ -168,8 +168,10 @@ def Test(argv, need_right_bracket):

      if bool_node is None:
        for e in b_parser.Error():
-          log("error %s", e)
-        log("Error parsing test/[ expression")
+          log("test: %s", e.UserErrorString())
+        # TODO: There should be a nice method to print argv.  And some way to
+        # point to the error.
+        log("Error parsing test/[ expression: %s", argv)
        return 2  # parse error is 2

  except util.ParseError as e:
diff --git a/core/util.py b/core/util.py
@@ -34,6 +34,7 @@ class _ErrorWithLocation(Exception):
  Formatting is in ui.PrintError.
  """
  def __init__(self, msg, *args, **kwargs):
+    Exception.__init__(self)
    self.msg = msg
    self.args = args
    # NOTE: We use a kwargs dict because Python 2 doesn't have keyword-only
@@ -46,8 +47,12 @@ def __init__(self, msg, *args, **kwargs):
    if kwargs:
      raise AssertionError('Invalid keyword args %s' % kwargs)

-  #def __repr__(self):
-  #  return '<%s %s %r %r %d>' % (self.msg, self.args, self.token, self.word, self.exit_status)
+  def __repr__(self):
+    return '<%s %s %r %r %s>' % (self.msg, self.args, self.token, self.word, self.exit_status)
+
+  def __str__(self):
+    # The default doesn't work very well?
+    return repr(self)

  def UserErrorString(self):
    return self.msg % self.args
diff --git a/osh/bool_parse.py b/osh/bool_parse.py
@@ -96,7 +96,8 @@ def _NextOne(self, lex_mode=lex_mode_e.DBRACKET):

    self.op_id = word.BoolId(self.cur_word)
    self.b_kind = LookupKind(self.op_id)
-    #print('---- word', self.cur_word, 'op_id', self.op_id, self.b_kind, lex_mode)
+    #log('--- word %s', self.cur_word)
+    #log('op_id %s %s %s', self.op_id, self.b_kind, lex_mode)
    return True

  def _Next(self, lex_mode=lex_mode_e.DBRACKET):
@@ -143,7 +144,9 @@ def ParseForBuiltin(self):

    node = self.ParseExpr()
    if self.op_id != Id.Eof_Real:
-      self.AddErrorContext("Trailing words in test", self.cur_word)
+      self.AddErrorContext(
+          'Unexpected trailing word in test expression: %s',
+          self.cur_word)
      return None

    return node
@@ -193,7 +196,6 @@ def ParseNegatedFactor(self):
      if not self._Next(): return None
      child = self.ParseFactor()
      if not child: return None
-      #return UnaryExprNode(Id.KW_Bang, child)
      return ast.LogicalNot(child)
    else:
      return self.ParseFactor()
@@ -220,7 +222,8 @@ def ParseFactor(self):
      t2_op_id = word.BoolId(t2)
      t2_b_kind = LookupKind(t2_op_id)

-      # Redir PUN for < and >
+      #log('t2 %s / t2_op_id %s / t2_b_kind %s', t2, t2_op_id, t2_b_kind)
+      # Redir pun for < and >, -a and -o pun
      if t2_b_kind in (Kind.BoolBinary, Kind.Redir):
        left = self.cur_word