From 837e8aa81afafa17e7bcb615df9775b092234b71 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 19 Apr 2023 20:42:10 +0900 Subject: [PATCH] revset: add substitution rule for nested descendants/children The substitution rule and tests are copied from ancestors/parents. The backend logic will be reimplemented later. For now, it naively repeats children(). --- lib/src/default_revset_engine.rs | 12 ++- lib/src/revset.rs | 125 ++++++++++++++++++++++++++----- lib/tests/test_revset.rs | 75 +++++++++++++++++++ 3 files changed, 191 insertions(+), 21 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 80a8fd4d9..7b9bee733 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -618,12 +618,16 @@ impl<'index> EvaluationContext<'index> { heads, generation_from_roots, } => { - let root_set = self.evaluate(roots)?; + let mut root_set = self.evaluate(roots)?; let head_set = self.evaluate(heads)?; - if generation_from_roots == &(1..2) { - Ok(Box::new(self.walk_children(&*root_set, &*head_set))) + // TODO: implement generic evaluation path for generation filter + for _ in 0..generation_from_roots.start { + root_set = Box::new(self.walk_children(&*root_set, &*head_set)); + } + if generation_from_roots.end == generation_from_roots.start + 1 { + Ok(root_set) } else { - assert_eq!(generation_from_roots, &GENERATION_RANGE_FULL); // TODO + assert_eq!(generation_from_roots.end, u64::MAX); // TODO let (dag_range_set, _) = self.collect_dag_range(&*root_set, &*head_set); Ok(Box::new(dag_range_set)) } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 8f3ca52f5..fe92cddd3 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -1516,8 +1516,20 @@ fn unfold_difference(expression: &Rc) -> TransformedExpression }) } -/// Transforms nested `ancestors()`/`parents()` like `h---`. -fn fold_ancestors(expression: &Rc) -> TransformedExpression { +/// Transforms nested `ancestors()`/`parents()`/`descendants()`/`children()` +/// like `h---`/`r+++`. +fn fold_generation(expression: &Rc) -> TransformedExpression { + fn add_generation(generation1: &Range, generation2: &Range) -> Range { + // For any (g1, g2) in (generation1, generation2), g1 + g2. + if generation1.is_empty() || generation2.is_empty() { + GENERATION_RANGE_EMPTY + } else { + let start = u64::saturating_add(generation1.start, generation2.start); + let end = u64::saturating_add(generation1.end, generation2.end - 1); + start..end + } + } + transform_expression_bottom_up(expression, |expression| match expression.as_ref() { RevsetExpression::Ancestors { heads, @@ -1530,20 +1542,28 @@ fn fold_ancestors(expression: &Rc) -> TransformedExpression { RevsetExpression::Ancestors { heads, generation: generation2, - } => { - // For any (g1, g2) in (generation1, generation2), g1 + g2. - let generation = if generation1.is_empty() || generation2.is_empty() { - GENERATION_RANGE_EMPTY - } else { - let start = u64::saturating_add(generation1.start, generation2.start); - let end = u64::saturating_add(generation1.end, generation2.end - 1); - start..end - }; - Some(Rc::new(RevsetExpression::Ancestors { - heads: heads.clone(), - generation, - })) - } + } => Some(Rc::new(RevsetExpression::Ancestors { + heads: heads.clone(), + generation: add_generation(generation1, generation2), + })), + _ => None, + } + } + RevsetExpression::Descendants { + roots, + generation: generation1, + } => { + match roots.as_ref() { + // (r+)+ -> descendants(descendants(r, 1), 1) -> descendants(r, 2) + // (r+): -> descendants(descendants(r, 1), ..) -> descendants(r, 1..) + // (r:)+ -> descendants(descendants(r, ..), 1) -> descendants(r, 1..) + RevsetExpression::Descendants { + roots, + generation: generation2, + } => Some(Rc::new(RevsetExpression::Descendants { + roots: roots.clone(), + generation: add_generation(generation1, generation2), + })), _ => None, } } @@ -1557,7 +1577,7 @@ fn fold_ancestors(expression: &Rc) -> TransformedExpression { pub fn optimize(expression: Rc) -> Rc { let expression = unfold_difference(&expression).unwrap_or(expression); let expression = fold_redundant_expression(&expression).unwrap_or(expression); - let expression = fold_ancestors(&expression).unwrap_or(expression); + let expression = fold_generation(&expression).unwrap_or(expression); let expression = internalize_filter(&expression).unwrap_or(expression); fold_difference(&expression).unwrap_or(expression) } @@ -3918,4 +3938,75 @@ mod tests { "### ); } + + #[test] + fn test_optimize_descendants() { + // Typical scenario: fold nested children() + insta::assert_debug_snapshot!(optimize(parse("foo++").unwrap()), @r###" + Descendants { + roots: CommitRef( + Symbol( + "foo", + ), + ), + generation: 2..3, + } + "###); + insta::assert_debug_snapshot!(optimize(parse("(foo+++):").unwrap()), @r###" + Descendants { + roots: CommitRef( + Symbol( + "foo", + ), + ), + generation: 3..18446744073709551615, + } + "###); + insta::assert_debug_snapshot!(optimize(parse("(foo:)+++").unwrap()), @r###" + Descendants { + roots: CommitRef( + Symbol( + "foo", + ), + ), + generation: 3..18446744073709551615, + } + "###); + + // 'foo+-' is not 'foo'. + insta::assert_debug_snapshot!(optimize(parse("foo+++-").unwrap()), @r###" + Ancestors { + heads: Descendants { + roots: CommitRef( + Symbol( + "foo", + ), + ), + generation: 3..4, + }, + generation: 1..2, + } + "###); + + // TODO: Inner Descendants can be folded into DagRange. Perhaps, we can rewrite + // 'x:y' to 'x: & :y' first, so the common substitution rule can handle both + // 'x+:y' and 'x+ & :y'. + insta::assert_debug_snapshot!(optimize(parse("(foo++):bar").unwrap()), @r###" + DagRange { + roots: Descendants { + roots: CommitRef( + Symbol( + "foo", + ), + ), + generation: 2..3, + }, + heads: CommitRef( + Symbol( + "bar", + ), + ), + } + "###); + } } diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index fcd11b93f..d5923b70b 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -778,6 +778,10 @@ fn test_evaluate_expression_children(use_git: bool) { .set_parents(vec![commit3.id().clone(), commit4.id().clone()]) .write() .unwrap(); + let commit6 = create_random_commit(mut_repo, &settings) + .set_parents(vec![commit5.id().clone()]) + .write() + .unwrap(); // Can find children of the root commit assert_eq!( @@ -808,6 +812,28 @@ fn test_evaluate_expression_children(use_git: bool) { vec![commit5.id().clone()] ); + // Can find children of children, which may be optimized to single query + assert_eq!( + resolve_commit_ids(mut_repo, "root++"), + vec![commit4.id().clone(), commit2.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, &format!("(root | {})++", commit1.id().hex())), + vec![ + commit5.id().clone(), + commit4.id().clone(), + commit3.id().clone(), + commit2.id().clone(), + ] + ); + assert_eq!( + resolve_commit_ids( + mut_repo, + &format!("({} | {})++", commit4.id().hex(), commit2.id().hex()) + ), + vec![commit6.id().clone(), commit5.id().clone()] + ); + // Empty root assert_eq!(resolve_commit_ids(mut_repo, "none()+"), vec![]); } @@ -1141,11 +1167,16 @@ fn test_evaluate_expression_descendants(use_git: bool) { .set_parents(vec![commit3.id().clone(), commit4.id().clone()]) .write() .unwrap(); + let commit6 = create_random_commit(mut_repo, &settings) + .set_parents(vec![commit5.id().clone()]) + .write() + .unwrap(); // The descendants of the root commit are all the commits in the repo assert_eq!( resolve_commit_ids(mut_repo, "root:"), vec![ + commit6.id().clone(), commit5.id().clone(), commit4.id().clone(), commit3.id().clone(), @@ -1159,11 +1190,55 @@ fn test_evaluate_expression_descendants(use_git: bool) { assert_eq!( resolve_commit_ids(mut_repo, &format!("{}:", commit2.id().hex())), vec![ + commit6.id().clone(), commit5.id().clone(), commit3.id().clone(), commit2.id().clone(), ] ); + + // Can find descendants of children or children of descendants, which may be + // optimized to single query + assert_eq!( + resolve_commit_ids(mut_repo, &format!("({}+):", commit1.id().hex())), + vec![ + commit6.id().clone(), + commit5.id().clone(), + commit4.id().clone(), + commit3.id().clone(), + commit2.id().clone(), + ] + ); + assert_eq!( + resolve_commit_ids(mut_repo, &format!("({}++):", commit1.id().hex())), + vec![ + commit6.id().clone(), + commit5.id().clone(), + commit3.id().clone(), + ] + ); + assert_eq!( + resolve_commit_ids( + mut_repo, + &format!("(({}|{}):)+", commit4.id().hex(), commit2.id().hex()), + ), + vec![ + commit6.id().clone(), + commit5.id().clone(), + commit3.id().clone(), + ] + ); + assert_eq!( + resolve_commit_ids( + mut_repo, + &format!("(({}|{})+):", commit4.id().hex(), commit2.id().hex()), + ), + vec![ + commit6.id().clone(), + commit5.id().clone(), + commit3.id().clone(), + ] + ); } #[test_case(false ; "local backend")]