Patchwork [5,of,5] revset: optimize out sort() to noop function according to ordering policy

login
register
mail settings
Submitter Yuya Nishihara
Date May 30, 2016, 3:11 p.m.
Message ID <30cdc763d2e6da828e2a.1464621095@mimosa>
Download mbox | patch
Permalink /patch/15263/
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 30cdc763d2e6da828e2af4fcbc818f722c82ad77
# Parent  723ed6b1d76b90b19f7211f0432c947c0113619e
revset: optimize out sort() to noop function according to ordering policy

See the previous patch for why. Unlike reverse(), we need a noop function
to validate arguments.
Sean Farley - June 1, 2016, 1:22 a.m.
Yuya Nishihara <yuya@tcha.org> writes:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1462250172 -32400
> #      Tue May 03 13:36:12 2016 +0900
> # Node ID 30cdc763d2e6da828e2af4fcbc818f722c82ad77
> # Parent  723ed6b1d76b90b19f7211f0432c947c0113619e
> revset: optimize out sort() to noop function according to ordering policy
>
> See the previous patch for why. Unlike reverse(), we need a noop function
> to validate arguments.

These look good to me; thanks for fixing this!
Yuya Nishihara - June 1, 2016, 12:38 p.m.
On Tue, 31 May 2016 18:22:48 -0700, Sean Farley wrote:
> Yuya Nishihara <yuya@tcha.org> writes:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1462250172 -32400
> > #      Tue May 03 13:36:12 2016 +0900
> > # Node ID 30cdc763d2e6da828e2af4fcbc818f722c82ad77
> > # Parent  723ed6b1d76b90b19f7211f0432c947c0113619e
> > revset: optimize out sort() to noop function according to ordering policy
> >
> > See the previous patch for why. Unlike reverse(), we need a noop function
> > to validate arguments.  
> 
> These look good to me; thanks for fixing this!

Thanks.

I noticed 'present()' isn't handled properly. I'll send a follow-up patch
if there are no serious problems in this series.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1896,6 +1896,12 @@  def sort(repo, subset, x):
             raise error.ParseError(_("unknown sort key %r") % fk)
     return baseset([c.rev() for c in ctxs])
 
+@predicate('_nosort(set[, [-]key...])', safe=True)
+def _nosort(repo, subset, x):
+    sort(repo, baseset(), x)  # validate arguments
+    args = getargsdict(x, 'sort', 'set keys')
+    return getset(repo, subset, args['set'])
+
 @predicate('subrepo([pattern])')
 def subrepo(repo, subset, x):
     """Changesets that add, modify or remove the given subrepo.  If no subrepo
@@ -2249,6 +2255,8 @@  def _optimize(x, small, order):
         wa, ta = _optimize(x[2], small, _defineorder)
         if f == 'reverse' and order != _defineorder:
             return wa, ta
+        if f == 'sort' and order != _defineorder:
+            return wa, ('func', ('symbol', '_nosort'), 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
@@ -1126,8 +1126,7 @@  Test order of revisions in compound expr
   (see hg help "revsets.x or y")
   [255]
 
- BROKEN: sort() will need different workaround since it modifies the flag
- of input set:
+ sort() should be optimized out if order doesn't matter:
 
   $ try --optimize '0:2 & sort(all(), -rev)'
   (and
@@ -1149,23 +1148,28 @@  Test order of revisions in compound expr
       ('symbol', '2')
       ('string', 'define'))
     (func
-      ('symbol', '_reorder')
-      (func
-        ('symbol', 'sort')
-        (list
-          (func
-            ('symbol', 'all')
-            None)
-          ('string', '-rev')))))
+      ('symbol', '_nosort')
+      (list
+        (func
+          ('symbol', 'all')
+          None)
+        ('string', '-rev'))))
   * 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 sort() to be optimized out:
+
+  $ log '0:2 & sort()'
+  hg: parse error: sort requires one or two arguments
+  [255]
+  $ log '0:2 & sort(all(), -invalid)'
+  hg: parse error: unknown sort key '-invalid'
+  [255]
 
  for 'A & f(B)', 'B' should not be affected by the order of 'A':