From 643de8d41b91c3c5de893b1ed539372f79b4e118 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Fri, 20 Sep 2024 19:06:01 +0200 Subject: [PATCH] interfaces: generalize constant folding for patterns The existing constant folding mechanism for sequences-of-literals is now generalized to inject the literal into the following item, possibly reducing the complexity of the tree at the cost of making the tree contain longer strings. The tests are not fully adjusted, as the render test measures precise indices that were too tedious to fix on Friday. Signed-off-by: Zygmunt Krynicki --- .../prompting/patterns/parse_internal_test.go | 16 ++++ interfaces/prompting/patterns/render.go | 95 ++++++++++++++----- .../patterns/render_internal_test.go | 75 ++++++++++++--- 3 files changed, 148 insertions(+), 38 deletions(-) diff --git a/interfaces/prompting/patterns/parse_internal_test.go b/interfaces/prompting/patterns/parse_internal_test.go index 93b96620213..6d465adcc6e 100644 --- a/interfaces/prompting/patterns/parse_internal_test.go +++ b/interfaces/prompting/patterns/parse_internal_test.go @@ -68,3 +68,19 @@ func (s *parseSuite) TestParse(c *C) { c.Assert(err, IsNil) c.Check(tree, DeepEquals, handMadeTree) } + +func (s *parseSuite) TestParseWithLeadingLiteralOptimization(c *C) { + pattern := "/{foo,bar}" + + handMadeTree := alt{ + literal("/foo"), + literal("/bar"), + } + + tokens, err := scan(pattern) + c.Assert(err, IsNil) + tree, err := parse(tokens) + c.Logf("Tree is: %v", tree) + c.Assert(err, IsNil) + c.Check(tree, DeepEquals, handMadeTree) +} diff --git a/interfaces/prompting/patterns/render.go b/interfaces/prompting/patterns/render.go index 90fba610597..c8b6b478a4e 100644 --- a/interfaces/prompting/patterns/render.go +++ b/interfaces/prompting/patterns/render.go @@ -22,7 +22,6 @@ package patterns import ( "bytes" "errors" - "strings" "github.com/snapcore/snapd/logger" ) @@ -57,6 +56,7 @@ type renderNode interface { NumVariants() int // nodeEqual returns true if two nodes are recursively equal. nodeEqual(renderNode) bool + prependLiteral(literal) renderNode } // renderAllVariants renders subsequent variants to a reusable buffer. @@ -109,6 +109,10 @@ func (n literal) nodeEqual(other renderNode) bool { return false } +func (n literal) prependLiteral(l literal) renderNode { + return l + n +} + func (n literal) Render(buf *bytes.Buffer, alreadyRendered int) int { if alreadyRendered > 0 { return alreadyRendered - len(n) @@ -176,44 +180,56 @@ func (n seq) nodeEqual(other renderNode) bool { // empty seq to a literal "", and reduces a seq with one element to that // element. func (n seq) optimize() (renderNode, error) { - var b strings.Builder - + // Constant-fold leading literal into whatever that follows it. + var prefix literal var newSeq seq - for _, item := range n { - if l, ok := item.(literal); ok { - if l == "" { - continue - } - - b.WriteString(string(l)) - } else { - if b.Len() > 0 { - newSeq = append(newSeq, literal(b.String())) - b.Reset() - } + if prefix != "" { + // XXX: prependItem may return modified item. + item = item.prependLiteral(prefix) + } - newSeq = append(newSeq, item) + if item, ok := item.(literal); ok { + prefix = item + continue } + + newSeq = append(newSeq, item) + prefix = "" } - if b.Len() > 0 { - newSeq = append(newSeq, literal(b.String())) - b.Reset() + if prefix != "" { + newSeq = append(newSeq, prefix) } + n = newSeq + // Reduce strength - switch len(newSeq) { + switch len(n) { case 0: return literal(""), nil case 1: - return newSeq[0], nil + return n[0], nil } - return newSeq, nil + return n, nil +} + +func (n seq) prependLiteral(l literal) renderNode { + if l == "" { + return n + } + + if len(n) == 0 { + return l + } + + n[0] = n[0].prependLiteral(l) + + return n } -// seqVariant is the variant state for a seqeunce of render nodes. +// seqVariant is the variant state for a sequence of render nodes. // // Each render node in the seq has a corresponding variant at the same index. // of the elements list. @@ -314,9 +330,24 @@ func (n alt) nodeEqual(other renderNode) bool { // optimize for alt eliminates equivalent items and reduces alts with one item // to that item. func (n alt) optimize() (renderNode, error) { - var seen []renderNode var newAlt alt + // This pass adds items that are alts back to us to unify the set of + // alts, effectively. + for _, item := range n { + if innerAlt, ok := item.(alt); ok { + newAlt = append(newAlt, innerAlt...) + } else { + newAlt = append(newAlt, item) + } + } + + n = newAlt + newAlt = nil + + var seen []renderNode + + // This pass eliminates duplicate entries. outer: for _, item := range n { for _, seenItem := range seen { @@ -342,6 +373,22 @@ outer: return newAlt, nil } +func (n alt) prependLiteral(l literal) renderNode { + if l == "" { + return n + } + + if len(n) == 0 { + return l + } + + for i := range n { + n[i] = n[i].prependLiteral(l) + } + + return n +} + // altVariant is the variant state for an set of alternative render nodes. type altVariant struct { alt alt // Alt associated with this variant state. diff --git a/interfaces/prompting/patterns/render_internal_test.go b/interfaces/prompting/patterns/render_internal_test.go index 2252515f335..42ebf6efdd0 100644 --- a/interfaces/prompting/patterns/render_internal_test.go +++ b/interfaces/prompting/patterns/render_internal_test.go @@ -190,22 +190,16 @@ func (s *renderSuite) TestOptimizeNodeEqualTrue(c *C) { }, { "/foo/{a,b}", - seq{ - literal("/foo/"), - alt{ - literal("a"), - literal("b"), - }, + alt{ + literal("/foo/a"), + literal("/foo/b"), }, }, { "/foo/{{a,b},{a,b}}", - seq{ - literal("/foo/"), - alt{ - literal("a"), - literal("b"), - }, + alt{ + literal("/foo/a"), + literal("/foo/b"), }, }, } { @@ -213,8 +207,8 @@ func (s *renderSuite) TestOptimizeNodeEqualTrue(c *C) { c.Assert(err, IsNil, Commentf("testCase: %+v", testCase)) parsed, err := parse(scanned) c.Assert(err, IsNil, Commentf("testCase: %+v", testCase)) - c.Check(parsed.nodeEqual(testCase.byHand), Equals, true, Commentf("testCase: %+v\nparsed: %+v", testCase, parsed)) - c.Check(testCase.byHand.nodeEqual(parsed), Equals, true, Commentf("testCase: %+v\nparsed: %+v", testCase, parsed)) + c.Check(parsed.nodeEqual(testCase.byHand), Equals, true, Commentf("testCase: %+v\nparsed: %#v", testCase, parsed)) + c.Check(testCase.byHand.nodeEqual(parsed), Equals, true, Commentf("testCase: %+v\nparsed: %#v", testCase, parsed)) } } @@ -281,3 +275,56 @@ func (s *renderSuite) TestOptimizeNodeEqualFalse(c *C) { c.Check(testCase.byHand.nodeEqual(parsed), Equals, false, Commentf("testCase: %+v\nparsed: %+v", testCase, parsed)) } } + +func (s *renderSuite) TestRenderAllVariantsConflicts(c *C) { + // all variants are the exact same path + for _, testCase := range []struct { + pattern string + expectedVariants []string + }{ + { + "/{fo{obar},foo{b{ar,ar},bar,{ba}r}}", + []string{"/foobar"}, + }, + { + "/{{foo/{bar,baz},123},{123,foo{/bar,/baz}}}", + []string{ + "/foo/bar", + "/foo/baz", + "/123", + }, + }, + { + "/{{a,DUP},{b,DUP}}", + []string{ + "/a", + "/DUP", + "/b", + }, + }, + { + "/{{foo,bar}/baz/{fizz,buzz},{foo/,bar/}baz{/fizz,/buzz}}", + []string{ + "/foo/baz/fizz", + "/foo/baz/buzz", + "/bar/baz/fizz", + "/bar/baz/buzz", + "/foo/baz/fizz", + "/foo/baz/buzz", + "/bar/baz/fizz", + "/bar/baz/buzz", + }, + }, + } { + scanned, err := scan(testCase.pattern) + c.Assert(err, IsNil) + parsed, err := parse(scanned) + c.Assert(err, IsNil) + variants := make([]string, 0, len(testCase.expectedVariants)) + renderAllVariants(parsed, func(index int, variant PatternVariant) { + variants = append(variants, variant.String()) + }) + c.Check(variants, DeepEquals, testCase.expectedVariants) + c.Check(variants, HasLen, parsed.NumVariants()) + } +}