Patchwork [2,of,2,STABLE] revset: prevent crash caused by empty group expression while optimizing "or"

login
register
mail settings
Submitter Yuya Nishihara
Date Aug. 9, 2015, 8:33 a.m.
Message ID <56a853ed34fbd7556296.1439109207@mimosa>
Download mbox | patch
Permalink /patch/10184/
State Accepted
Headers show

Comments

Yuya Nishihara - Aug. 9, 2015, 8:33 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1439104181 -32400
#      Sun Aug 09 16:09:41 2015 +0900
# Branch stable
# Node ID 56a853ed34fbd755629692cec73555913e8bcb42
# Parent  56d3f5a9c5021efa42454e30a2748aab1e2924d2
revset: prevent crash caused by empty group expression while optimizing "or"

An empty group expression "()" generates None in AST, so it should be tested
before destructuring a tuple.

"A | ()" is still evaluated to an error because I'm not sure whether "()"
represents an empty set or an empty expression (= a unit value). They are
identical in "or" operation, but they should be evaluated differently in
"and" operation.

  expression  empty set  unit value
  ----------  ---------  ----------
  ()          {}         A
  A & ()      {}         A
  A | ()      A          A
Pierre-Yves David - Aug. 10, 2015, 7:22 a.m.
On 08/09/2015 01:33 AM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1439104181 -32400
> #      Sun Aug 09 16:09:41 2015 +0900
> # Branch stable
> # Node ID 56a853ed34fbd755629692cec73555913e8bcb42
> # Parent  56d3f5a9c5021efa42454e30a2748aab1e2924d2
> revset: prevent crash caused by empty group expression while optimizing "or"

Pushed to the clowncopter, bad error messages are superior to crashes.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2280,7 +2280,7 @@  def optimize(x, small):
             del ss[:]
         for y in x[1:]:
             w, t = optimize(y, False)
-            if t[0] == 'string' or t[0] == 'symbol':
+            if t is not None and (t[0] == 'string' or t[0] == 'symbol'):
                 ss.append((w, t))
                 continue
             flushss()
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -1144,6 +1144,20 @@  test that chained `or` operations make b
   4
   5
 
+no crash by empty group "()" while optimizing `or` operations
+
+  $ try --optimize '0|()'
+  (or
+    ('symbol', '0')
+    (group
+      None))
+  * optimized:
+  (or
+    ('symbol', '0')
+    None)
+  hg: parse error: missing argument
+  [255]
+
 test that chained `or` operations never eat up stack (issue4624)
 (uses `0:1` instead of `0` to avoid future optimization of trivial revisions)