Skip to content

Commit

Permalink
Add Options to prioritize some set of options over another
Browse files Browse the repository at this point in the history
Added API:
	Default() Option
	FilterPriority(opts ...Option) Option

FilterPriority returns a new Option where an option, opts[i],
is only evaluated if no fundamental options remain after applying all filters
in all prior options, opts[:i].

In order to prevent further options from being evaluated, the Default option
can be used to ensure that some fundamental option remains.

Suppose you have a value tree T, where T1 and T2 are sub-trees within T.
Prior to the addition of FilterPriority, it was impossible to do certain things.

Example 1: You could not make the following compose together nicely.
	* Have a set of options OT1 to affect only values under T1.
	* Have a set of options OT2 to affect only values under T2.
	* Have a set of options OT to affect only T, but not values under T1 and T2.
	* Have a set of options O to affect all other values, but no those in T
	  (and by extension those in T1 and T2).
Solution 1:
	FilterPriority(
		// Since T1 and T2 do not overlap, they could be placed within the
		// same priority level by grouping them in an Options group.
		FilterTree(T1, OT1),
		FilterTree(T2, OT2),
		FilterTree(T, OT),
		O,
	)

Example 2: You could not make the following compose together nicely.
	* Have a set of options O apply on all nodes except those in T1 and T2.
	* Instead, we want the default behavior of cmp on T1 and T2.
Solution 2:
	FilterPriority(
		// Here we show how to group T1 and T2 together to be on the same
		// priority level.
		Options{
			FilterTree(T1, Default()),
			FilterTree(T2, Default()),
		},
		O,
	)

Example 3: You have this: type MyStruct struct { *pb.MyMessage; ... }
	* Generally, you want to use Comparer(proto.Equal) to ensure
	  that all proto.Messages within the struct are properly compared.
	  However, this type has an embedded proto (generally a bad idea),
	  which causes the MyStruct to satisfy the proto.Message interface
	  and unintentionally causes Equal to use proto.Equal on MyStruct,
	  which crashes.
	* How can you have Comparer(proto.Equal) apply to all other
	  proto.Message without applying just to MyStruct?
Solution 3:
	FilterPriority(
		// Only for MyStruct, use the default behavior of Equal,
		// which is to recurse into the structure of MyStruct.
		FilterPath(func(p Path) bool {
			return p.Last().Type() == reflect.TypeOf(MyStruct{})
		}, Default()),

		// Use proto.Equal for all other cases of proto.Message.
		Comparer(proto.Equal),
	)
  • Loading branch information
dsnet committed Aug 9, 2017
1 parent 8099a97 commit 5b174e0
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 27 deletions.
9 changes: 5 additions & 4 deletions cmp/compare.go
Expand Up @@ -49,11 +49,12 @@ var nothing = reflect.Value{}
// • If two values are not of the same type, then they are never equal
// and the overall result is false.
//
// • Let S be the set of all Ignore, Transformer, and Comparer options that
// remain after applying all path filters, value filters, and type filters.
// • Let S be the set of all Ignore, Transformer, Comparer, and Default options
// that remain after applying all implicit and explicit filters.
// If at least one Ignore exists in S, then the comparison is ignored.
// If the number of Transformer and Comparer options in S is greater than one,
// then Equal panics because it is ambiguous which option to use.
// If S contains multiple Default options, they are coalesced into one Default.
// If any Transformer, Comparer, or Default options coexist in S,
// then Equal panics because it is ambiguous which to use.
// If S contains a single Transformer, then use that to transform the current
// values and recursively call Equal on the output values.
// If S contains a single Comparer, then use that to compare the current values.
Expand Down
107 changes: 84 additions & 23 deletions cmp/options.go
Expand Up @@ -14,11 +14,11 @@ import (
)

// Option configures for specific behavior of Equal and Diff. In particular,
// the fundamental Option functions (Ignore, Transformer, and Comparer),
// the fundamental options (Ignore, Transformer, Comparer, and Default),
// configure how equality is determined.
//
// The fundamental options may be composed with filters (FilterPath and
// FilterValues) to control the scope over which they are applied.
// The fundamental options may be composed with filters (FilterPath, FilterValues,
// and FilterPriority) to control the scope over which they are applied.
//
// The cmp/cmpopts package provides helper functions for creating options that
// may be used with Equal and Diff.
Expand All @@ -33,7 +33,7 @@ type Option interface {
}

// applicableOption represents the following types:
// Fundamental: ignore | invalid | *comparer | *transformer
// Fundamental: noop | ignore | invalid | *comparer | *transformer
// Grouping: Options
type applicableOption interface {
Option
Expand All @@ -44,8 +44,8 @@ type applicableOption interface {
}

// coreOption represents the following types:
// Fundamental: ignore | invalid | *comparer | *transformer
// Filters: *pathFilter | *valuesFilter
// Fundamental: noop | ignore | invalid | *comparer | *transformer
// Filters: *pathFilter | *valuesFilter | *priorityFilter
type coreOption interface {
Option
isCore()
Expand All @@ -57,28 +57,28 @@ func (core) isCore() {}

// Options is a list of Option values that also satisfies the Option interface.
// Helper comparison packages may return an Options value when packing multiple
// Option values into a single Option. When this package processes an Options,
// it will be implicitly expanded into a flat list.
//
// Applying a filter on an Options is equivalent to applying that same filter
// on all individual options held within.
// Option values into a single Option. When this package processes Options
// or nested Options, it will be implicitly expanded into a flat set.
type Options []Option

func (opts Options) filter(s *state, vx, vy reflect.Value, t reflect.Type) (out applicableOption) {
for _, opt := range opts {
switch opt := opt.filter(s, vx, vy, t); opt.(type) {
case ignore:
return ignore{} // Only ignore can short-circuit evaluation
return ignore{} // Highest precedence; can short-circuit filtering
case invalid:
out = invalid{} // Takes precedence over comparer or transformer
case *comparer, *transformer, Options:
out = invalid{} // Second highest precedence
case noop, *comparer, *transformer, Options:
switch out.(type) {
case nil:
out = opt
case invalid:
// Keep invalid
case *comparer, *transformer, Options:
out = Options{out, opt} // Conflicting comparers or transformers
case noop, *comparer, *transformer, Options:
if opt == (noop{}) && out == (noop{}) {
break // Coelesce redundant Default together
}
out = Options{out, opt} // Conflicting Comparer, Tranformer, or Default
}
}
}
Expand All @@ -87,7 +87,7 @@ func (opts Options) filter(s *state, vx, vy reflect.Value, t reflect.Type) (out

func (opts Options) apply(s *state, _, _ reflect.Value) bool {
const warning = "ambiguous set of applicable options"
const help = "consider using filters to ensure at most one Comparer or Transformer may apply"
const help = "consider using filters to ensure at most one Comparer, Transformer, or Default may apply"
var ss []string
for _, opt := range flattenOptions(nil, opts) {
ss = append(ss, fmt.Sprint(opt))
Expand All @@ -104,11 +104,12 @@ func (opts Options) String() string {
return fmt.Sprintf("Options{%s}", strings.Join(ss, ", "))
}

// FilterPath returns a new Option where opt is only evaluated if filter f
// FilterPath returns a new Option where opt is only applicable if filter f
// returns true for the current Path in the value tree.
//
// The option passed in may be an Ignore, Transformer, Comparer, Options, or
// a previously filtered Option.
// The Option passed in may be a filtered option (via the Filter functions),
// fundamental option (like Ignore, Transformer, Comparer, or Default), or
// Options group containing elements of the former.
func FilterPath(f func(Path) bool, opt Option) Option {
if f == nil {
panic("invalid path filter function")
Expand Down Expand Up @@ -137,7 +138,7 @@ func (f pathFilter) String() string {
return fmt.Sprintf("FilterPath(%s, %v)", fn, f.opt)
}

// FilterValues returns a new Option where opt is only evaluated if filter f,
// FilterValues returns a new Option where opt is only applicable if filter f,
// which is a function of the form "func(T, T) bool", returns true for the
// current pair of values being compared. If the type of the values is not
// assignable to T, then this filter implicitly returns false.
Expand All @@ -148,8 +149,9 @@ func (f pathFilter) String() string {
// If T is an interface, it is possible that f is called with two values with
// different concrete types that both implement T.
//
// The option passed in may be an Ignore, Transformer, Comparer, Options, or
// a previously filtered Option.
// The Option passed in may be a filtered option (via the Filter functions),
// fundamental option (like Ignore, Transformer, Comparer, or Default), or
// Options group containing elements of the former.
func FilterValues(f interface{}, opt Option) Option {
v := reflect.ValueOf(f)
if !function.IsType(v.Type(), function.ValueFilter) || v.IsNil() {
Expand Down Expand Up @@ -187,6 +189,65 @@ func (f valuesFilter) String() string {
return fmt.Sprintf("FilterValues(%s, %v)", fn, f.opt)
}

// FilterPriority returns a new Option where an option, opts[i],
// is only applicable if no fundamental options remain after applying all filters
// in all prior options, opts[:i].
//
// In order to prevent further options from being applicable, the Default option
// can be used to ensure that some fundamental option remains.
//
// The Option passed in may be a filtered option (via the Filter functions),
// fundamental option (like Ignore, Transformer, Comparer, or Default), or
// Options group containing elements of the former.
func FilterPriority(opts ...Option) Option {
var newOpts []Option
for _, opt := range opts {
if opt := normalizeOption(opt); opt != nil {
newOpts = append(newOpts, opt)
}
}
if len(newOpts) > 0 {
return &priorityFilter{opts: newOpts}
}
return nil
}

type priorityFilter struct {
core
opts []Option
}

func (f priorityFilter) filter(s *state, vx, vy reflect.Value, t reflect.Type) applicableOption {
for _, opt := range f.opts {
if opt := opt.filter(s, vx, vy, t); opt != nil {
return opt
}
}
return nil
}

func (f priorityFilter) String() string {
var ss []string
for _, opt := range f.opts {
ss = append(ss, fmt.Sprint(opt))
}
return fmt.Sprintf("FilterPriority(%s)", strings.Join(ss, ", "))
}

// Default is an Option that configures Equal to stop processing options and
// to proceed to the next evaluation rule (i.e., checking for the Equal method).
// This value is intended to be combined with FilterPriority to act as a
// sentinel type that prevents other options from being applicable.
// It is an error to pass an unfiltered Default option to Equal.
func Default() Option { return noop{} }

type noop struct{ core }

func (noop) isFiltered() bool { return false }
func (noop) filter(_ *state, _, _ reflect.Value, _ reflect.Type) applicableOption { return noop{} }
func (noop) apply(_ *state, _, _ reflect.Value) bool { return false }
func (noop) String() string { return "Default()" }

// Ignore is an Option that causes all comparisons to be ignored.
// This value is intended to be combined with FilterPath or FilterValues.
// It is an error to pass an unfiltered Ignore option to Equal.
Expand Down

0 comments on commit 5b174e0

Please sign in to comment.