Patchwork [3,of,6,V3] revset: forward ordering requirement to argument of present()

login
register
mail settings
Submitter Yuya Nishihara
Date June 17, 2016, 2:45 p.m.
Message ID <2654c4c9d89d9a301059.1466174729@mimosa>
Download mbox | patch
Permalink /patch/15536/
State Superseded
Delegated to: Martin von Zweigbergk
Headers show

Comments

Yuya Nishihara - June 17, 2016, 2:45 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1464782044 -32400
#      Wed Jun 01 20:54:04 2016 +0900
# Node ID 2654c4c9d89d9a301059f4c8473a0cfe32e94f10
# Parent  a99114251d721a469991574105d1a0167f4fb162
revset: forward ordering requirement to argument of present()

present() is special in that it forwards the argument set with no modification.
We could handle this case by marking present() as "ordering" symbol, but we
have better option. This patch introduces new category "noop" to forward the
ordering requirement to arguments.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2322,6 +2322,12 @@  methods = {
     '_hexlist',
 ])
 
+# Symbols which return the evaluated set with no modification. In other words,
+# functions that do 'return getset(repo, subset, x)'.
+_noopsymbols = set([
+    'present',
+])
+
 # Constants for ordering requirement, used in _optimize():
 #
 # If 'define', any future functions and operations can change the ordering of
@@ -2497,7 +2503,13 @@  def _optimize(x, small, order):
         return sum(ws), (op,) + ts
     elif op == 'func':
         f = getstring(x[1], _("not a symbol"))
-        wa, ta = _optimize(x[2], small, _defineorder)
+        # 'f' may have its own ordering requirement, but 'noop' function is
+        # known to return the argument set with no modification, so forward
+        # the requirement in that case.
+        d = _defineorder
+        if f in _noopsymbols:
+            d = order
+        wa, ta = _optimize(x[2], small, d)
         if f in ("author branch closed date desc file grep keyword "
                  "outgoing user"):
             w = 10 # slow
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -978,6 +978,69 @@  Test order of revisions in compound expr
   1
   0
 
+ because 'present()' does nothing other than suppressing an error, the
+ ordering requirement should be forwarded to the inner expression, and
+ '_reorder' should be inserted only when necessary:
+
+  $ try --optimize '2:0 & present(0 + 1 + 2)'
+  (and
+    (range
+      ('symbol', '2')
+      ('symbol', '0'))
+    (func
+      ('symbol', 'present')
+      (or
+        ('symbol', '0')
+        ('symbol', '1')
+        ('symbol', '2'))))
+  * optimized:
+  (and
+    (range
+      ('symbol', '2')
+      ('symbol', '0'))
+    (func
+      ('symbol', 'present')
+      (func
+        ('symbol', '_reorder')
+        (func
+          ('symbol', '_list')
+          ('string', '0\x001\x002')))))
+  * set:
+  <filteredset
+    <spanset- 0:2>,
+    <baseset [0, 1, 2]>>
+  2
+  1
+  0
+
+  $ try --optimize '2:0 & present(all())'
+  (and
+    (range
+      ('symbol', '2')
+      ('symbol', '0'))
+    (func
+      ('symbol', 'present')
+      (func
+        ('symbol', 'all')
+        None)))
+  * optimized:
+  (and
+    (range
+      ('symbol', '2')
+      ('symbol', '0'))
+    (func
+      ('symbol', 'present')
+      (func
+        ('symbol', 'all')
+        None)))
+  * set:
+  <filteredset
+    <spanset- 0:2>,
+    <spanset+ 0:9>>
+  2
+  1
+  0
+
  '_reorder' should be omitted if order doesn't matter:
 
   $ try --optimize '2:0 & not (0 + 1)'