Patchwork [4,of,5] revset: optimize out reverse() according to ordering policy

login
register
mail settings
Submitter Yuya Nishihara
Date May 30, 2016, 3:11 p.m.
Message ID <723ed6b1d76b90b19f72.1464621094@mimosa>
Download mbox | patch
Permalink /patch/15262/
State Superseded
Headers show

Comments

Yuya Nishihara - May 30, 2016, 3:11 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1462250172 -32400
#      Tue May 03 13:36:12 2016 +0900
# Node ID 723ed6b1d76b90b19f7211f0432c947c0113619e
# Parent  7ed64e1d13c699211ad82431af51f513d2aafbbb
revset: optimize out reverse() according to ordering policy

Because smartset.reverse() may modify the underlying subset, it should be
called only if the set can define the ordering.

In the following example, 'a' and 'c' is the same object, so 'b.reverse()'
would reverse 'a' unexpectedly.

  # '0:2 & reverse(all())'
  <filteredset
    <spanset- 0:2>,    # a
    <filteredset       # b
      <spanset- 0:2>,  # c
      <spanset+ 0:9>>>

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2247,6 +2247,8 @@  def _optimize(x, small, order):
     elif op == 'func':
         f = getstring(x[1], _("not a symbol"))
         wa, ta = _optimize(x[2], small, _defineorder)
+        if f == 'reverse' and order != _defineorder:
+            return wa, ta
         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
@@ -1087,8 +1087,7 @@  Test order of revisions in compound expr
       <baseset [0, 1]>>>
   2
 
- BROKEN: reverse() and sort() will need different workaround since they
- modify the flag of input set:
+ reverse() should be optimized out if order doesn't matter:
 
   $ try --optimize '0:2 & reverse(all())'
   (and
@@ -1107,21 +1106,28 @@  Test order of revisions in compound expr
       ('symbol', '2')
       ('string', 'define'))
     (func
-      ('symbol', '_reorder')
-      (func
-        ('symbol', 'reverse')
-        (func
-          ('symbol', 'all')
-          None))))
+      ('symbol', 'all')
+      None))
   * set:
   <filteredset
-    <spanset- 0:2>,
-    <filteredset
-      <spanset- 0:2>,
-      <spanset+ 0:9>>>
+    <spanset+ 0:2>,
+    <spanset+ 0:9>>
+  0
+  1
   2
-  1
-  0
+
+ invalid argument for reverse() to be optimized out:
+
+  $ log '0:2 & reverse()'
+  hg: parse error: missing argument
+  [255]
+  $ log '0:2 & reverse(0, 1)'
+  hg: parse error: can't use a list in this context
+  (see hg help "revsets.x or y")
+  [255]
+
+ BROKEN: sort() will need different workaround since it modifies the flag
+ of input set:
 
   $ try --optimize '0:2 & sort(all(), -rev)'
   (and