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

login
register
mail settings
Submitter Yuya Nishihara
Date June 17, 2016, 2:45 p.m.
Message ID <249e5cd2f9a97e17872e.1466174731@mimosa>
Download mbox | patch
Permalink /patch/15538/
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 1462250172 -32400
#      Tue May 03 13:36:12 2016 +0900
# Node ID 249e5cd2f9a97e17872e78b6f1e16e709e01bca3
# Parent  6274b62bdd47e6a2b3497355ab092ce2e7d6d780
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
@@ -2515,6 +2515,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