Patchwork [STABLE] revset: flatten chained 'list' operations (aka function args) (issue5072)

login
register
mail settings
Submitter Yuya Nishihara
Date Feb. 4, 2016, 3:08 p.m.
Message ID <016bccc8a86965cba08d.1454598532@mimosa>
Download mbox | patch
Permalink /patch/12977/
State Accepted
Headers show

Comments

Yuya Nishihara - Feb. 4, 2016, 3:08 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1454424589 -32400
#      Tue Feb 02 23:49:49 2016 +0900
# Branch stable
# Node ID 016bccc8a86965cba08d3d6ad77638dfb81cd5e0
# Parent  c58c4683e2b71a18d6b82f2e8c6e7607b1079bef
revset: flatten chained 'list' operations (aka function args) (issue5072)

Internal _matchfiles() function can take bunch of arguments, which would
lead to a maximum recursion depth error. This patch avoids the excessive
stack use by flattening 'list' nodes beforehand.

Since getlist() no longer takes a nested 'list' nodes, _parsealiasdecl()
also needs to flatten argument list, "aliasname($1, $2, ...)".
Augie Fackler - Feb. 5, 2016, 10:10 p.m.
On Fri, Feb 05, 2016 at 12:08:52AM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1454424589 -32400
> #      Tue Feb 02 23:49:49 2016 +0900
> # Branch stable
> # Node ID 016bccc8a86965cba08d3d6ad77638dfb81cd5e0
> # Parent  c58c4683e2b71a18d6b82f2e8c6e7607b1079bef
> revset: flatten chained 'list' operations (aka function args) (issue5072)

Queued for stable, thanks.

>
> Internal _matchfiles() function can take bunch of arguments, which would
> lead to a maximum recursion depth error. This patch avoids the excessive
> stack use by flattening 'list' nodes beforehand.
>
> Since getlist() no longer takes a nested 'list' nodes, _parsealiasdecl()
> also needs to flatten argument list, "aliasname($1, $2, ...)".
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -319,7 +319,7 @@ def getlist(x):
>      if not x:
>          return []
>      if x[0] == 'list':
> -        return getlist(x[1]) + [x[2]]
> +        return list(x[1:])
>      return [x]
>
>  def getargs(x, min, max, err):
> @@ -448,7 +448,7 @@ def orset(repo, subset, *xs):
>  def notset(repo, subset, x):
>      return subset - getset(repo, subset, x)
>
> -def listset(repo, subset, a, b):
> +def listset(repo, subset, *xs):
>      raise error.ParseError(_("can't use a list in this context"),
>                             hint=_('see hg help "revsets.x or y"'))
>
> @@ -2252,7 +2252,7 @@ def optimize(x, small):
>          return o[0], (op, o[1])
>      elif op == 'group':
>          return optimize(x[1], small)
> -    elif op in 'dagrange range list parent ancestorspec':
> +    elif op in 'dagrange range parent ancestorspec':
>          if op == 'parent':
>              # x^:y means (x^) : y, not x ^ (:y)
>              post = ('parentpost', x[1])
> @@ -2264,6 +2264,9 @@ def optimize(x, small):
>          wa, ta = optimize(x[1], small)
>          wb, tb = optimize(x[2], small)
>          return wa + wb, (op, ta, tb)
> +    elif op == 'list':
> +        ws, ts = zip(*(optimize(y, small) for y in x[1:]))
> +        return sum(ws), (op,) + ts
>      elif op == 'func':
>          f = getstring(x[1], _("not a symbol"))
>          wa, ta = optimize(x[2], small)
> @@ -2365,6 +2368,7 @@ def _parsealiasdecl(decl):
>          tree, pos = p.parse(_tokenizealias(decl))
>          if (pos != len(decl)):
>              raise error.ParseError(_('invalid token'), pos)
> +        tree = parser.simplifyinfixops(tree, ('list',))
>
>          if isvalidsymbol(tree):
>              # "name = ...." style
> @@ -2455,7 +2459,7 @@ def _parsealiasdefn(defn, args):
>      tree, pos = p.parse(tokenizedefn(defn))
>      if pos != len(defn):
>          raise error.ParseError(_('invalid token'), pos)
> -    return parser.simplifyinfixops(tree, ('or',))
> +    return parser.simplifyinfixops(tree, ('list', 'or'))
>
>  class revsetalias(object):
>      # whether own `error` information is already shown or not.
> @@ -2586,7 +2590,7 @@ def parse(spec, lookup=None):
>      tree, pos = p.parse(tokenize(spec, lookup=lookup))
>      if pos != len(spec):
>          raise error.ParseError(_("invalid token"), pos)
> -    return parser.simplifyinfixops(tree, ('or',))
> +    return parser.simplifyinfixops(tree, ('list', 'or'))
>
>  def posttreebuilthook(tree, repo):
>      # hook for extensions to execute code on the optimized tree
> diff --git a/tests/test-glog.t b/tests/test-glog.t
> --- a/tests/test-glog.t
> +++ b/tests/test-glog.t
> @@ -1602,11 +1602,9 @@ Test falling back to slow path for non-e
>      (func
>        ('symbol', '_matchfiles')
>        (list
> -        (list
> -          (list
> -            ('string', 'r:')
> -            ('string', 'd:relpath'))
> -          ('string', 'p:a'))
> +        ('string', 'r:')
> +        ('string', 'd:relpath')
> +        ('string', 'p:a')
>          ('string', 'p:c'))))
>
>  Test multiple --include/--exclude/paths
> @@ -1617,19 +1615,13 @@ Test multiple --include/--exclude/paths
>      (func
>        ('symbol', '_matchfiles')
>        (list
> -        (list
> -          (list
> -            (list
> -              (list
> -                (list
> -                  (list
> -                    ('string', 'r:')
> -                    ('string', 'd:relpath'))
> -                  ('string', 'p:a'))
> -                ('string', 'p:e'))
> -              ('string', 'i:a'))
> -            ('string', 'i:e'))
> -          ('string', 'x:b'))
> +        ('string', 'r:')
> +        ('string', 'd:relpath')
> +        ('string', 'p:a')
> +        ('string', 'p:e')
> +        ('string', 'i:a')
> +        ('string', 'i:e')
> +        ('string', 'x:b')
>          ('string', 'x:e'))))
>
>  Test glob expansion of pats
> @@ -1668,9 +1660,8 @@ Test --follow on a directory
>        (func
>          ('symbol', '_matchfiles')
>          (list
> -          (list
> -            ('string', 'r:')
> -            ('string', 'd:relpath'))
> +          ('string', 'r:')
> +          ('string', 'd:relpath')
>            ('string', 'p:dir')))))
>    $ hg up -q tip
>
> @@ -1693,9 +1684,8 @@ Test --follow and patterns
>        (func
>          ('symbol', '_matchfiles')
>          (list
> -          (list
> -            ('string', 'r:')
> -            ('string', 'd:relpath'))
> +          ('string', 'r:')
> +          ('string', 'd:relpath')
>            ('string', 'p:glob:*')))))
>
>  Test --follow on a single rename
> @@ -1836,9 +1826,8 @@ Test "set:..." and parent revision
>      (func
>        ('symbol', '_matchfiles')
>        (list
> -        (list
> -          ('string', 'r:')
> -          ('string', 'd:relpath'))
> +        ('string', 'r:')
> +        ('string', 'd:relpath')
>          ('string', 'p:set:copied()'))))
>    $ testlog --include "set:copied()"
>    []
> @@ -1846,9 +1835,8 @@ Test "set:..." and parent revision
>      (func
>        ('symbol', '_matchfiles')
>        (list
> -        (list
> -          ('string', 'r:')
> -          ('string', 'd:relpath'))
> +        ('string', 'r:')
> +        ('string', 'd:relpath')
>          ('string', 'i:set:copied()'))))
>    $ testlog -r "sort(file('set:copied()'), -rev)"
>    ["sort(file('set:copied()'), -rev)"]
> @@ -1865,9 +1853,8 @@ Test --removed
>      (func
>        ('symbol', '_matchfiles')
>        (list
> -        (list
> -          ('string', 'r:')
> -          ('string', 'd:relpath'))
> +        ('string', 'r:')
> +        ('string', 'd:relpath')
>          ('string', 'p:a'))))
>    $ testlog --removed --follow a
>    []
> @@ -1879,9 +1866,8 @@ Test --removed
>        (func
>          ('symbol', '_matchfiles')
>          (list
> -          (list
> -            ('string', 'r:')
> -            ('string', 'd:relpath'))
> +          ('string', 'r:')
> +          ('string', 'd:relpath')
>            ('string', 'p:a')))))
>
>  Test --patch and --stat with --follow and --follow-first
> @@ -2271,9 +2257,8 @@ Test subdir
>      (func
>        ('symbol', '_matchfiles')
>        (list
> -        (list
> -          ('string', 'r:')
> -          ('string', 'd:relpath'))
> +        ('string', 'r:')
> +        ('string', 'd:relpath')
>          ('string', 'p:.'))))
>    $ testlog ../b
>    []
> diff --git a/tests/test-revset.t b/tests/test-revset.t
> --- a/tests/test-revset.t
> +++ b/tests/test-revset.t
> @@ -1168,6 +1168,14 @@ test ',' in `_list`
>    hg: parse error: can't use a list in this context
>    (see hg help "revsets.x or y")
>    [255]
> +  $ try '0,1,2'
> +  (list
> +    ('symbol', '0')
> +    ('symbol', '1')
> +    ('symbol', '2'))
> +  hg: parse error: can't use a list in this context
> +  (see hg help "revsets.x or y")
> +  [255]
>
>  test that chained `or` operations make balanced addsets
>
> @@ -1717,13 +1725,12 @@ test chained `or` operations are flatten
>    (func
>      ('symbol', 'chainedorops')
>      (list
> -      (list
> -        (range
> -          ('symbol', '0')
> -          ('symbol', '1'))
> -        (range
> -          ('symbol', '1')
> -          ('symbol', '2')))
> +      (range
> +        ('symbol', '0')
> +        ('symbol', '1'))
> +      (range
> +        ('symbol', '1')
> +        ('symbol', '2'))
>        (range
>          ('symbol', '2')
>          ('symbol', '3'))))
> @@ -1877,9 +1884,8 @@ far away.
>    (func
>      ('symbol', 'rs')
>      (list
> -      (list
> -        ('symbol', '2')
> -        ('symbol', 'data'))
> +      ('symbol', '2')
> +      ('symbol', 'data')
>        ('symbol', '7')))
>    hg: parse error: invalid number of arguments: 3
>    [255]
> @@ -1887,13 +1893,11 @@ far away.
>    (func
>      ('symbol', 'rs4')
>      (list
> -      (list
> -        (list
> -          (or
> -            ('symbol', '2')
> -            ('symbol', '3'))
> -          ('symbol', 'x'))
> -        ('symbol', 'x'))
> +      (or
> +        ('symbol', '2')
> +        ('symbol', '3'))
> +      ('symbol', 'x')
> +      ('symbol', 'x')
>        ('symbol', 'date')))
>    (func
>      ('symbol', 'reverse')
> @@ -2055,11 +2059,9 @@ tests for concatenation of strings/symbo
>    (func
>      ('symbol', 'cat4')
>      (list
> -      (list
> -        (list
> -          ('symbol', '278')
> -          ('string', '5f5'))
> -        ('symbol', '1ee'))
> +      ('symbol', '278')
> +      ('string', '5f5')
> +      ('symbol', '1ee')
>        ('string', 'ce5')))
>    (_concat
>      (_concat
> _______________________________________________
> 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
@@ -319,7 +319,7 @@  def getlist(x):
     if not x:
         return []
     if x[0] == 'list':
-        return getlist(x[1]) + [x[2]]
+        return list(x[1:])
     return [x]
 
 def getargs(x, min, max, err):
@@ -448,7 +448,7 @@  def orset(repo, subset, *xs):
 def notset(repo, subset, x):
     return subset - getset(repo, subset, x)
 
-def listset(repo, subset, a, b):
+def listset(repo, subset, *xs):
     raise error.ParseError(_("can't use a list in this context"),
                            hint=_('see hg help "revsets.x or y"'))
 
@@ -2252,7 +2252,7 @@  def optimize(x, small):
         return o[0], (op, o[1])
     elif op == 'group':
         return optimize(x[1], small)
-    elif op in 'dagrange range list parent ancestorspec':
+    elif op in 'dagrange range parent ancestorspec':
         if op == 'parent':
             # x^:y means (x^) : y, not x ^ (:y)
             post = ('parentpost', x[1])
@@ -2264,6 +2264,9 @@  def optimize(x, small):
         wa, ta = optimize(x[1], small)
         wb, tb = optimize(x[2], small)
         return wa + wb, (op, ta, tb)
+    elif op == 'list':
+        ws, ts = zip(*(optimize(y, small) for y in x[1:]))
+        return sum(ws), (op,) + ts
     elif op == 'func':
         f = getstring(x[1], _("not a symbol"))
         wa, ta = optimize(x[2], small)
@@ -2365,6 +2368,7 @@  def _parsealiasdecl(decl):
         tree, pos = p.parse(_tokenizealias(decl))
         if (pos != len(decl)):
             raise error.ParseError(_('invalid token'), pos)
+        tree = parser.simplifyinfixops(tree, ('list',))
 
         if isvalidsymbol(tree):
             # "name = ...." style
@@ -2455,7 +2459,7 @@  def _parsealiasdefn(defn, args):
     tree, pos = p.parse(tokenizedefn(defn))
     if pos != len(defn):
         raise error.ParseError(_('invalid token'), pos)
-    return parser.simplifyinfixops(tree, ('or',))
+    return parser.simplifyinfixops(tree, ('list', 'or'))
 
 class revsetalias(object):
     # whether own `error` information is already shown or not.
@@ -2586,7 +2590,7 @@  def parse(spec, lookup=None):
     tree, pos = p.parse(tokenize(spec, lookup=lookup))
     if pos != len(spec):
         raise error.ParseError(_("invalid token"), pos)
-    return parser.simplifyinfixops(tree, ('or',))
+    return parser.simplifyinfixops(tree, ('list', 'or'))
 
 def posttreebuilthook(tree, repo):
     # hook for extensions to execute code on the optimized tree
diff --git a/tests/test-glog.t b/tests/test-glog.t
--- a/tests/test-glog.t
+++ b/tests/test-glog.t
@@ -1602,11 +1602,9 @@  Test falling back to slow path for non-e
     (func
       ('symbol', '_matchfiles')
       (list
-        (list
-          (list
-            ('string', 'r:')
-            ('string', 'd:relpath'))
-          ('string', 'p:a'))
+        ('string', 'r:')
+        ('string', 'd:relpath')
+        ('string', 'p:a')
         ('string', 'p:c'))))
 
 Test multiple --include/--exclude/paths
@@ -1617,19 +1615,13 @@  Test multiple --include/--exclude/paths
     (func
       ('symbol', '_matchfiles')
       (list
-        (list
-          (list
-            (list
-              (list
-                (list
-                  (list
-                    ('string', 'r:')
-                    ('string', 'd:relpath'))
-                  ('string', 'p:a'))
-                ('string', 'p:e'))
-              ('string', 'i:a'))
-            ('string', 'i:e'))
-          ('string', 'x:b'))
+        ('string', 'r:')
+        ('string', 'd:relpath')
+        ('string', 'p:a')
+        ('string', 'p:e')
+        ('string', 'i:a')
+        ('string', 'i:e')
+        ('string', 'x:b')
         ('string', 'x:e'))))
 
 Test glob expansion of pats
@@ -1668,9 +1660,8 @@  Test --follow on a directory
       (func
         ('symbol', '_matchfiles')
         (list
-          (list
-            ('string', 'r:')
-            ('string', 'd:relpath'))
+          ('string', 'r:')
+          ('string', 'd:relpath')
           ('string', 'p:dir')))))
   $ hg up -q tip
 
@@ -1693,9 +1684,8 @@  Test --follow and patterns
       (func
         ('symbol', '_matchfiles')
         (list
-          (list
-            ('string', 'r:')
-            ('string', 'd:relpath'))
+          ('string', 'r:')
+          ('string', 'd:relpath')
           ('string', 'p:glob:*')))))
 
 Test --follow on a single rename
@@ -1836,9 +1826,8 @@  Test "set:..." and parent revision
     (func
       ('symbol', '_matchfiles')
       (list
-        (list
-          ('string', 'r:')
-          ('string', 'd:relpath'))
+        ('string', 'r:')
+        ('string', 'd:relpath')
         ('string', 'p:set:copied()'))))
   $ testlog --include "set:copied()"
   []
@@ -1846,9 +1835,8 @@  Test "set:..." and parent revision
     (func
       ('symbol', '_matchfiles')
       (list
-        (list
-          ('string', 'r:')
-          ('string', 'd:relpath'))
+        ('string', 'r:')
+        ('string', 'd:relpath')
         ('string', 'i:set:copied()'))))
   $ testlog -r "sort(file('set:copied()'), -rev)"
   ["sort(file('set:copied()'), -rev)"]
@@ -1865,9 +1853,8 @@  Test --removed
     (func
       ('symbol', '_matchfiles')
       (list
-        (list
-          ('string', 'r:')
-          ('string', 'd:relpath'))
+        ('string', 'r:')
+        ('string', 'd:relpath')
         ('string', 'p:a'))))
   $ testlog --removed --follow a
   []
@@ -1879,9 +1866,8 @@  Test --removed
       (func
         ('symbol', '_matchfiles')
         (list
-          (list
-            ('string', 'r:')
-            ('string', 'd:relpath'))
+          ('string', 'r:')
+          ('string', 'd:relpath')
           ('string', 'p:a')))))
 
 Test --patch and --stat with --follow and --follow-first
@@ -2271,9 +2257,8 @@  Test subdir
     (func
       ('symbol', '_matchfiles')
       (list
-        (list
-          ('string', 'r:')
-          ('string', 'd:relpath'))
+        ('string', 'r:')
+        ('string', 'd:relpath')
         ('string', 'p:.'))))
   $ testlog ../b
   []
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -1168,6 +1168,14 @@  test ',' in `_list`
   hg: parse error: can't use a list in this context
   (see hg help "revsets.x or y")
   [255]
+  $ try '0,1,2'
+  (list
+    ('symbol', '0')
+    ('symbol', '1')
+    ('symbol', '2'))
+  hg: parse error: can't use a list in this context
+  (see hg help "revsets.x or y")
+  [255]
 
 test that chained `or` operations make balanced addsets
 
@@ -1717,13 +1725,12 @@  test chained `or` operations are flatten
   (func
     ('symbol', 'chainedorops')
     (list
-      (list
-        (range
-          ('symbol', '0')
-          ('symbol', '1'))
-        (range
-          ('symbol', '1')
-          ('symbol', '2')))
+      (range
+        ('symbol', '0')
+        ('symbol', '1'))
+      (range
+        ('symbol', '1')
+        ('symbol', '2'))
       (range
         ('symbol', '2')
         ('symbol', '3'))))
@@ -1877,9 +1884,8 @@  far away.
   (func
     ('symbol', 'rs')
     (list
-      (list
-        ('symbol', '2')
-        ('symbol', 'data'))
+      ('symbol', '2')
+      ('symbol', 'data')
       ('symbol', '7')))
   hg: parse error: invalid number of arguments: 3
   [255]
@@ -1887,13 +1893,11 @@  far away.
   (func
     ('symbol', 'rs4')
     (list
-      (list
-        (list
-          (or
-            ('symbol', '2')
-            ('symbol', '3'))
-          ('symbol', 'x'))
-        ('symbol', 'x'))
+      (or
+        ('symbol', '2')
+        ('symbol', '3'))
+      ('symbol', 'x')
+      ('symbol', 'x')
       ('symbol', 'date')))
   (func
     ('symbol', 'reverse')
@@ -2055,11 +2059,9 @@  tests for concatenation of strings/symbo
   (func
     ('symbol', 'cat4')
     (list
-      (list
-        (list
-          ('symbol', '278')
-          ('string', '5f5'))
-        ('symbol', '1ee'))
+      ('symbol', '278')
+      ('string', '5f5')
+      ('symbol', '1ee')
       ('string', 'ce5')))
   (_concat
     (_concat