Patchwork [7,of,7] revset: fix order of nested '_(|int|hex)list' expression (BC)

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 13, 2016, 4:13 p.m.
Message ID <75ba18a83289dcff7b65.1473783208@mimosa>
Download mbox | patch
Permalink /patch/16606/
State Accepted
Headers show

Comments

Yuya Nishihara - Sept. 13, 2016, 4:13 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1466934088 -32400
#      Sun Jun 26 18:41:28 2016 +0900
# Node ID 75ba18a83289dcff7b652a6db9334bcbd01f198f
# Parent  14af6142b21b0f3e3cf742410d4e7adb92ff4c76
revset: fix order of nested '_(|int|hex)list' expression (BC)

This fixes the order of 'x & (y + z)' where 'y' and 'z' are trivial, and the
other uses of _list()-family functions. The original functions are renamed to
'_ordered(|int|hex)list' to say clearly that they do not follow the subset
ordering.
Augie Fackler - Sept. 13, 2016, 8:37 p.m.
On Wed, Sep 14, 2016 at 01:13:28AM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1466934088 -32400
> #      Sun Jun 26 18:41:28 2016 +0900
> # Node ID 75ba18a83289dcff7b652a6db9334bcbd01f198f
> # Parent  14af6142b21b0f3e3cf742410d4e7adb92ff4c76
> revset: fix order of nested '_(|int|hex)list' expression (BC)

Nice work. I've queued all of these.

>
> This fixes the order of 'x & (y + z)' where 'y' and 'z' are trivial, and the
> other uses of _list()-family functions. The original functions are renamed to
> '_ordered(|int|hex)list' to say clearly that they do not follow the subset
> ordering.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -2253,9 +2253,7 @@ def wdir(repo, subset, x):
>          return baseset([node.wdirrev])
>      return baseset()
>
> -# for internal use
> -@predicate('_list', safe=True)
> -def _list(repo, subset, x):
> +def _orderedlist(repo, subset, x):
>      s = getstring(x, "internal error")
>      if not s:
>          return baseset()
> @@ -2284,8 +2282,15 @@ def _list(repo, subset, x):
>      return baseset(ls)
>
>  # for internal use
> -@predicate('_intlist', safe=True)
> -def _intlist(repo, subset, x):
> +@predicate('_list', safe=True, takeorder=True)
> +def _list(repo, subset, x, order):
> +    if order == followorder:
> +        # slow path to take the subset order
> +        return subset & _orderedlist(repo, fullreposet(repo), x)
> +    else:
> +        return _orderedlist(repo, subset, x)
> +
> +def _orderedintlist(repo, subset, x):
>      s = getstring(x, "internal error")
>      if not s:
>          return baseset()
> @@ -2294,8 +2299,15 @@ def _intlist(repo, subset, x):
>      return baseset([r for r in ls if r in s])
>
>  # for internal use
> -@predicate('_hexlist', safe=True)
> -def _hexlist(repo, subset, x):
> +@predicate('_intlist', safe=True, takeorder=True)
> +def _intlist(repo, subset, x, order):
> +    if order == followorder:
> +        # slow path to take the subset order
> +        return subset & _orderedintlist(repo, fullreposet(repo), x)
> +    else:
> +        return _orderedintlist(repo, subset, x)
> +
> +def _orderedhexlist(repo, subset, x):
>      s = getstring(x, "internal error")
>      if not s:
>          return baseset()
> @@ -2304,6 +2316,15 @@ def _hexlist(repo, subset, x):
>      s = subset
>      return baseset([r for r in ls if r in s])
>
> +# for internal use
> +@predicate('_hexlist', safe=True, takeorder=True)
> +def _hexlist(repo, subset, x, order):
> +    if order == followorder:
> +        # slow path to take the subset order
> +        return subset & _orderedhexlist(repo, fullreposet(repo), x)
> +    else:
> +        return _orderedhexlist(repo, subset, x)
> +
>  methods = {
>      "range": rangeset,
>      "dagrange": dagrange,
> diff --git a/tests/test-revset.t b/tests/test-revset.t
> --- a/tests/test-revset.t
> +++ b/tests/test-revset.t
> @@ -1281,11 +1281,12 @@ ordering defined by it.
>        follow)
>      define)
>    * set:
> -  <baseset [0, 1, 2]>
> +  <filteredset
> +    <spanset- 0:2>,
> +    <baseset [0, 1, 2]>>
> +  2
> +  1
>    0
> -  1
> -  2
> - BROKEN: should be '2 1 0'
>
>   'A + B' should take the ordering of the left expression:
>
> @@ -1350,7 +1351,7 @@ ordering defined by it.
>    * set:
>    <filteredset
>      <spanset- 0:2>,
> -    <baseset [0, 1, 2]>>
> +    <baseset+ [0, 1, 2]>>
>    2
>    1
>    0
> @@ -1405,11 +1406,12 @@ ordering defined by it.
>        follow)
>      define)
>    * set:
> -  <baseset [0, 1, 2]>
> +  <filteredset
> +    <spanset- 0:2>,
> +    <baseset [0, 1, 2]>>
> +  2
> +  1
>    0
> -  1
> -  2
> - BROKEN: should be '2 1 0'
>
>    $ trylist --optimize --bin '%ln & 2:0' `hg log -T '{node} ' -r0+2+1`
>    (and
> @@ -1436,6 +1438,53 @@ ordering defined by it.
>    2
>    1
>
> + '_list' should not go through the slow follow-order path if order doesn't
> + matter:
> +
> +  $ try -p optimized '2:0 & not (0 + 1)'
> +  * optimized:
> +  (difference
> +    (range
> +      ('symbol', '2')
> +      ('symbol', '0')
> +      define)
> +    (func
> +      ('symbol', '_list')
> +      ('string', '0\x001')
> +      any)
> +    define)
> +  * set:
> +  <filteredset
> +    <spanset- 0:2>,
> +    <not
> +      <baseset [0, 1]>>>
> +  2
> +
> +  $ try -p optimized '2:0 & not (0:2 & (0 + 1))'
> +  * optimized:
> +  (difference
> +    (range
> +      ('symbol', '2')
> +      ('symbol', '0')
> +      define)
> +    (and
> +      (range
> +        ('symbol', '0')
> +        ('symbol', '2')
> +        any)
> +      (func
> +        ('symbol', '_list')
> +        ('string', '0\x001')
> +        any)
> +      any)
> +    define)
> +  * set:
> +  <filteredset
> +    <spanset- 0:2>,
> +    <not
> +      <baseset [0, 1]>>>
> +  2
> +
>   'present()' should do nothing other than suppressing an error:
>
>    $ try --optimize '2:0 & present(0 + 1 + 2)'
> @@ -1662,9 +1711,10 @@ ordering defined by it.
>      <spanset- 0:2>>
>    1
>
> - 'A & B' can be rewritten as 'B & A' by weight, but the ordering rule should
> - be determined before the optimization (i.e. 'B' should take the ordering of
> - 'A'):
> + 'A & B' can be rewritten as 'B & A' by weight, but that's fine as long as
> + the ordering rule is determined before the rewrite; in this example,
> + 'B' follows the order of the initial set, which is the same order as 'A'
> + since 'A' also follows the order:
>
>    $ try --optimize 'contains("glob:*") & (2 + 0 + 1)'
>    (and
> @@ -1690,12 +1740,14 @@ ordering defined by it.
>      define)
>    * set:
>    <filteredset
> -    <baseset [2, 0, 1]>,
> +    <baseset+ [0, 1, 2]>,
>      <contains 'glob:*'>>
> -  2
>    0
>    1
> - BROKEN: should be '0 1 2'
> +  2
> +
> + and in this example, 'A & B' is rewritten as 'B & A', but 'A' overrides
> + the order appropriately:
>
>    $ try --optimize 'reverse(contains("glob:*")) & (0 + 2 + 1)'
>    (and
> @@ -1726,12 +1778,11 @@ ordering defined by it.
>      define)
>    * set:
>    <filteredset
> -    <baseset [1, 2, 0]>,
> +    <baseset- [0, 1, 2]>,
>      <contains 'glob:*'>>
> +  2
>    1
> -  2
>    0
> - BROKEN: should be '2 1 0'
>
>  test sort revset
>  --------------------------------------------
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2253,9 +2253,7 @@  def wdir(repo, subset, x):
         return baseset([node.wdirrev])
     return baseset()
 
-# for internal use
-@predicate('_list', safe=True)
-def _list(repo, subset, x):
+def _orderedlist(repo, subset, x):
     s = getstring(x, "internal error")
     if not s:
         return baseset()
@@ -2284,8 +2282,15 @@  def _list(repo, subset, x):
     return baseset(ls)
 
 # for internal use
-@predicate('_intlist', safe=True)
-def _intlist(repo, subset, x):
+@predicate('_list', safe=True, takeorder=True)
+def _list(repo, subset, x, order):
+    if order == followorder:
+        # slow path to take the subset order
+        return subset & _orderedlist(repo, fullreposet(repo), x)
+    else:
+        return _orderedlist(repo, subset, x)
+
+def _orderedintlist(repo, subset, x):
     s = getstring(x, "internal error")
     if not s:
         return baseset()
@@ -2294,8 +2299,15 @@  def _intlist(repo, subset, x):
     return baseset([r for r in ls if r in s])
 
 # for internal use
-@predicate('_hexlist', safe=True)
-def _hexlist(repo, subset, x):
+@predicate('_intlist', safe=True, takeorder=True)
+def _intlist(repo, subset, x, order):
+    if order == followorder:
+        # slow path to take the subset order
+        return subset & _orderedintlist(repo, fullreposet(repo), x)
+    else:
+        return _orderedintlist(repo, subset, x)
+
+def _orderedhexlist(repo, subset, x):
     s = getstring(x, "internal error")
     if not s:
         return baseset()
@@ -2304,6 +2316,15 @@  def _hexlist(repo, subset, x):
     s = subset
     return baseset([r for r in ls if r in s])
 
+# for internal use
+@predicate('_hexlist', safe=True, takeorder=True)
+def _hexlist(repo, subset, x, order):
+    if order == followorder:
+        # slow path to take the subset order
+        return subset & _orderedhexlist(repo, fullreposet(repo), x)
+    else:
+        return _orderedhexlist(repo, subset, x)
+
 methods = {
     "range": rangeset,
     "dagrange": dagrange,
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -1281,11 +1281,12 @@  ordering defined by it.
       follow)
     define)
   * set:
-  <baseset [0, 1, 2]>
+  <filteredset
+    <spanset- 0:2>,
+    <baseset [0, 1, 2]>>
+  2
+  1
   0
-  1
-  2
- BROKEN: should be '2 1 0'
 
  'A + B' should take the ordering of the left expression:
 
@@ -1350,7 +1351,7 @@  ordering defined by it.
   * set:
   <filteredset
     <spanset- 0:2>,
-    <baseset [0, 1, 2]>>
+    <baseset+ [0, 1, 2]>>
   2
   1
   0
@@ -1405,11 +1406,12 @@  ordering defined by it.
       follow)
     define)
   * set:
-  <baseset [0, 1, 2]>
+  <filteredset
+    <spanset- 0:2>,
+    <baseset [0, 1, 2]>>
+  2
+  1
   0
-  1
-  2
- BROKEN: should be '2 1 0'
 
   $ trylist --optimize --bin '%ln & 2:0' `hg log -T '{node} ' -r0+2+1`
   (and
@@ -1436,6 +1438,53 @@  ordering defined by it.
   2
   1
 
+ '_list' should not go through the slow follow-order path if order doesn't
+ matter:
+
+  $ try -p optimized '2:0 & not (0 + 1)'
+  * optimized:
+  (difference
+    (range
+      ('symbol', '2')
+      ('symbol', '0')
+      define)
+    (func
+      ('symbol', '_list')
+      ('string', '0\x001')
+      any)
+    define)
+  * set:
+  <filteredset
+    <spanset- 0:2>,
+    <not
+      <baseset [0, 1]>>>
+  2
+
+  $ try -p optimized '2:0 & not (0:2 & (0 + 1))'
+  * optimized:
+  (difference
+    (range
+      ('symbol', '2')
+      ('symbol', '0')
+      define)
+    (and
+      (range
+        ('symbol', '0')
+        ('symbol', '2')
+        any)
+      (func
+        ('symbol', '_list')
+        ('string', '0\x001')
+        any)
+      any)
+    define)
+  * set:
+  <filteredset
+    <spanset- 0:2>,
+    <not
+      <baseset [0, 1]>>>
+  2
+
  'present()' should do nothing other than suppressing an error:
 
   $ try --optimize '2:0 & present(0 + 1 + 2)'
@@ -1662,9 +1711,10 @@  ordering defined by it.
     <spanset- 0:2>>
   1
 
- 'A & B' can be rewritten as 'B & A' by weight, but the ordering rule should
- be determined before the optimization (i.e. 'B' should take the ordering of
- 'A'):
+ 'A & B' can be rewritten as 'B & A' by weight, but that's fine as long as
+ the ordering rule is determined before the rewrite; in this example,
+ 'B' follows the order of the initial set, which is the same order as 'A'
+ since 'A' also follows the order:
 
   $ try --optimize 'contains("glob:*") & (2 + 0 + 1)'
   (and
@@ -1690,12 +1740,14 @@  ordering defined by it.
     define)
   * set:
   <filteredset
-    <baseset [2, 0, 1]>,
+    <baseset+ [0, 1, 2]>,
     <contains 'glob:*'>>
-  2
   0
   1
- BROKEN: should be '0 1 2'
+  2
+
+ and in this example, 'A & B' is rewritten as 'B & A', but 'A' overrides
+ the order appropriately:
 
   $ try --optimize 'reverse(contains("glob:*")) & (0 + 2 + 1)'
   (and
@@ -1726,12 +1778,11 @@  ordering defined by it.
     define)
   * set:
   <filteredset
-    <baseset [1, 2, 0]>,
+    <baseset- [0, 1, 2]>,
     <contains 'glob:*'>>
+  2
   1
-  2
   0
- BROKEN: should be '2 1 0'
 
 test sort revset
 --------------------------------------------