[PATCH 7 of 7 V2] revset: optimize out sort() to noop function according to ordering policy
Martin von Zweigbergk
martinvonz at google.com
Fri Jun 10 13:00:37 EDT 2016
On Fri, Jun 3, 2016 at 8:02 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1462250172 -32400
> # Tue May 03 13:36:12 2016 +0900
> # Node ID c4c434b91960e089a4fd28fae019280f49e4d2ab
> # Parent 57640fa6414ce96c8e4946a5f4862ce2688f9571
> 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.
>
> 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])
>
> + at predicate('_nosort(set[, [-]key...])', safe=True)
> +def _nosort(repo, subset, x):
> + sort(repo, baseset(), x) # validate arguments
sort() ends with "return baseset([c.rev() for c in ctxs])". Does that
evaluate the set? I know there's some laziness in revsets, but that
looks suspicious to me. Just checking; you probably know this code
better.
> + 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
> @@ -2092,6 +2098,7 @@ methods = {
> # functions that do 'return getset(repo, subset, x)'.
> _noopsymbols = set([
> 'present',
> + '_nosort',
> ])
>
> # constants for ordering policy, used in _optimize():
> @@ -2260,6 +2267,8 @@ def _optimize(x, small, order):
> wa, ta = _optimize(x[2], small, d)
> 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
> @@ -1191,8 +1191,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
> @@ -1214,23 +1213,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':
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list