D451: revset: remove order information from tree

[yuja (Yuya Nishihara)]

yuja added a comment.


  > How about making `registrar.revsetpredicate` conservative? If it sees any
  >  predicate registered with the old API, Disable `anyorder` optimization and
  >  change it to `followorder` in runtime?
  
  Sounds unnecessarily complicated. How fast is the `anyorder` set in real-world
  queries? Is that worth introducing more "if"s and corresponding tests?
  
  We can apply `anyorder` optimization to inner queries even if we decided to not
  optimize `_flipand()`. For example, think `ancestors(complex_query)`, where
  `complex_query` can be computed in `anyorder` constraint because we can be
  sure the order of head revisions doesn't matter. OTOH, `_flipand(x, y)` is hard
  because `x` is passed to arbitrary sub-expression `y`.
  
  >> As I said, most revset predicates have no explicit order. I introduced the
  >>  order flag in order to work around a few exceptions, which are `x:y`,
  >>  `x + y`, `sort()` and `reverse()`, IIRC. A strong "define" is exceptional.
  > 
  > I feel it inconsistent if `x:y` has a strong order but `x::y` does not.
  
  It would be surprising if `x::parent` suddenly starts listing `parent`'s children
  in reverse order.
  
  > I also want to change `p1`, etc's define order as mentioned above.
  
  I don't like this idea because it would make things more inconsistent.
  For instance, `branch(a + b)` could be `branch(a) + branch(b)`, and `ancestors(a:b)`
  could be `ancestors(a) + ... + ancestors(b)`, but is that really what we want?
  
  >> So, is it really make sense to revamp the revset ordering rules? I don't
  >>  think so. I generally like this series, but -1 for bringing "any" order
  >>  everywhere.
  > 
  > I think in general, the new code is more explicit and better testable.
  >  I'd like to move forward and not get blocked by legacy code.
  > 
  > I understand "anyorder" in the old code is almost a mistake. But with
  >  the new code, and suppose no legacy code is used, "anyoder" is safe.
  >  It seems to be simple (only used in a few places), less error-prone
  >  (core hg has and encourages strong defineorder), and do have value
  >  for optimization. I'd like to keep it. I can gate it by a config
  >  option but I don't think that's necessary if we make registrar handle
  >  it automatically.
  
  I doubt if it is really error-prone. We'll have to make sure that a revset
  function returns a set in the right "defined" order (or an unordered set
  which will be sorted implicitly by `intersect()`.)
  
  The current rule is IMHO, simpler. Almost all revsets are in the same
  order unless they are explicitly sorted or concatenated.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D451

To: quark, #hg-reviewers
Cc: martinvonz, yuja, mercurial-devel