[PATCH 2 of 7 V2] revset: teach optimize if current expression should define/follow order

[Yuya Nishihara]

On Thu, 9 Jun 2016 23:43:57 -0400, Augie Fackler wrote:
> On Tue, Jun 07, 2016 at 10:16:27PM +0900, Yuya Nishihara wrote:
> > On Mon, 6 Jun 2016 12:21:23 -0400, Augie Fackler wrote:
> > > On Sat, Jun 04, 2016 at 12:02:25AM +0900, Yuya Nishihara wrote:
> > > > # HG changeset patch
> > > > # User Yuya Nishihara <yuya at tcha.org>
> > > > # Date 1455627736 -32400
> > > > #      Tue Feb 16 22:02:16 2016 +0900
> > > > # Node ID f3c0deae1fd126d7340929c0c0612b88a4f5aa0e
> > > > # Parent  d200ad9058574d91a2b2b7a438602cb34b03ddbe
> > > > revset: teach optimize if current expression should define/follow order
> > > >
> > > > New flag 'order' will be used to fix a couple of ordering bugs, such as:
> > > >
> > > >  a) 'x & (y | z)' disregards the order of 'x' (issue 5100)
> > > >  b) 'x & y:z' is listed from 'y' to 'z'
> > > >  c) 'x & y' can be rewritten as 'y & x' if weight(x) > weight(y)
> > > >
> > > > (c) is a hidden bug of optimize(), which could be exposed if we've fixed
> > > > (a) and (b) in some way. (a) and (b) are not bugs of optimize(), but they
> > > > can be addressed well in optimize() since optimize() knows the context.
> > > >
> > > > 'order' is tri-state. It starts with 'define', and shifts to 'follow' by
> > > > 'x & y'. It changes back to 'define' on function call 'f(x)' or function-like
> > > > operation 'x (f) y' because 'f' may have its own ordering requirement for 'x'
> > > > and 'y'. The state 'any' will allow us to avoid extra cost that would be
> > > > necessary to constrain ordering where it isn't important, 'not x'. For details,
> > > > see also the tests added by the next patch.
> > >
> > > I'm going to duplicate what you said and see if I understand correctly:
> > >
> > > If 'order' is set to 'define', then any future binary operator can
> > > change the ordering of the entries in the set. If 'order' is set to
> > > 'follow', then any future binary operators should take the ordering
> > > specified by the first operand to the binary operator. Right?
> >
> >  s/any future binary operators/any future functions and operations/
> >  s/the first operand to the binary operator/the first operand to the '&'/
> >
> > For instance, "X & (Y | Z)" is evaluated as "or(y(x()), z(x()))". In this
> > case, "x()" can change the order of the entries in the set, but "y()", "z()"
> > and "or()" shouldn't. Because "or()" is known to change the order, we'll need
> > to fix the result of "or()".
> >
> > > I'm not sure I follow 'any' yet, but I haven't looked at that patch either.
> >
> > 'any' means the order doesn't matter. For "not X", "x()" may change the order
> > of the entries in the initial set. So for "not (x | y)", we don't need to fix
> > the result of "or()". And "not sort(x)" can be optimized to "not x".
> >
> 
> Ah, okay. Could I persuade you to send an updated version of this with
> the above clarification in comments near the relevant constants? I
> think that would greatly help future code archaeology.

Ok, my last explanation is not perfect, but it will be better than nothing.
I'll resend after Martijn's changes are queued.