Skip to content

Commit

Permalink
Add implicit filter to Transformers (#29)
Browse files Browse the repository at this point in the history
Declaring a transformer "func(T) T" where T is a concrete type
is a common transformation. However, this is currently problematic
as the transformation now infinitely applies to itself recursively.

In order to allow this form of transformation, add a simple
(but subtle) filter to Transformers where a Transformer can only
apply if that specific Transformer does not already exist within
the tail of the current Path since the last non-Transform step.

This rule does not prevent more advance usages of Transformers
where the user *does* want the Transformer to apply recursively
to previously transformed output, since those situations are
almost always followed by some path step that is *not* a
transformation (e.g., pointer indirect, slice indexing, etc).
  • Loading branch information
dsnet committed Nov 3, 2017
1 parent 7ffe192 commit 9823290
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 18 deletions.
10 changes: 3 additions & 7 deletions cmp/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1633,15 +1633,11 @@ func (gs germSorter) Swap(i, j int) { gs[i], gs[j] = gs[j], gs[i] }
func project2Tests() []test {
const label = "Project2"

sortGerms := cmp.FilterValues(func(x, y []*pb.Germ) bool {
ok1 := sort.IsSorted(germSorter(x))
ok2 := sort.IsSorted(germSorter(y))
return !ok1 || !ok2
}, cmp.Transformer("Sort", func(in []*pb.Germ) []*pb.Germ {
sortGerms := cmp.Transformer("Sort", func(in []*pb.Germ) []*pb.Germ {
out := append([]*pb.Germ(nil), in...) // Make copy
sort.Sort(germSorter(out))
return out
}))
})

equalDish := cmp.Comparer(func(x, y *ts.Dish) bool {
if x == nil || y == nil {
Expand Down Expand Up @@ -1739,7 +1735,7 @@ func project2Tests() []test {
{teststructs.GermBatch}.DirtyGerms[17]:
-: <non-existent>
+: []*testprotos.Germ{s"germ1"}
{teststructs.GermBatch}.DirtyGerms[18][2->?]:
Sort({teststructs.GermBatch}.DirtyGerms[18])[2->?]:
-: s"germ4"
+: <non-existent>
{teststructs.GermBatch}.DishMap[1]:
Expand Down
9 changes: 2 additions & 7 deletions cmp/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,16 +258,11 @@ func ExampleOption_equalEmpty() {
// This example is for demonstrative purposes; use cmpopts.SortSlices instead.
func ExampleOption_sortedSlice() {
// This Transformer sorts a []int.
// Since the transformer transforms []int into []int, there is problem where
// this is recursively applied forever. To prevent this, use a FilterValues
// to first check for the condition upon which the transformer ought to apply.
trans := cmp.FilterValues(func(x, y []int) bool {
return !sort.IntsAreSorted(x) || !sort.IntsAreSorted(y)
}, cmp.Transformer("Sort", func(in []int) []int {
trans := cmp.Transformer("Sort", func(in []int) []int {
out := append([]int(nil), in...) // Copy input to avoid mutating it
sort.Ints(out)
return out
}))
})

x := struct{ Ints []int }{[]int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}}
y := struct{ Ints []int }{[]int{2, 8, 0, 9, 6, 1, 4, 7, 3, 5}}
Expand Down
18 changes: 14 additions & 4 deletions cmp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,12 @@ func (invalid) apply(s *state, _, _ reflect.Value) bool {
// The transformer f must be a function "func(T) R" that converts values of
// type T to those of type R and is implicitly filtered to input values
// assignable to T. The transformer must not mutate T in any way.
// If T and R are the same type, an additional filter must be applied to
// act as the base case to prevent an infinite recursion applying the same
// transform to itself (see the SortedSlice example).
//
// To help prevent some cases of infinite recursive cycles applying the
// same transform to the output of itself (e.g., in the case where the
// input and output types are the same), an implicit filter is added such that
// a transformer is applicable only if that exact transformer is not already
// in the tail of the Path since the last non-Transform step.
//
// The name is a user provided label that is used as the Transform.Name in the
// transformation PathStep. If empty, an arbitrary name is used.
Expand Down Expand Up @@ -248,7 +251,14 @@ type transformer struct {

func (tr *transformer) isFiltered() bool { return tr.typ != nil }

func (tr *transformer) filter(_ *state, _, _ reflect.Value, t reflect.Type) applicableOption {
func (tr *transformer) filter(s *state, _, _ reflect.Value, t reflect.Type) applicableOption {
for i := len(s.curPath) - 1; i >= 0; i-- {
if t, ok := s.curPath[i].(*transform); !ok {
break // Hit most recent non-Transform step
} else if tr == t.trans {
return nil // Cannot directly use same Transform
}
}
if tr.typ == nil || t.AssignableTo(tr.typ) {
return tr
}
Expand Down

0 comments on commit 9823290

Please sign in to comment.