Patchwork [6,of,7,V2] revset: optimize out reverse() according to ordering policy

login
register
mail settings
Submitter Yuya Nishihara
Date June 3, 2016, 3:02 p.m.
Message ID <57640fa6414ce96c8e49.1464966149@mimosa>
Download mbox | patch
Permalink /patch/15377/
State Changes Requested
Delegated to: Augie Fackler
Headers show

Comments

Yuya Nishihara - June 3, 2016, 3:02 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1462250172 -32400
#      Tue May 03 13:36:12 2016 +0900
# Node ID 57640fa6414ce96c8e4946a5f4862ce2688f9571
# Parent  1ede480ae4adfb333351811bb1040698c4016650
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
@@ -2258,6 +2258,8 @@  def _optimize(x, small, order):
         if f in _noopsymbols:
             d = order
         wa, ta = _optimize(x[2], small, d)
+        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
@@ -1152,8 +1152,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
@@ -1172,21 +1171,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