Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

De-virtualize interfaces for specialized diffing #254

Merged
merged 1 commit into from Mar 3, 2021
Merged

De-virtualize interfaces for specialized diffing #254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions cmp/compare_test.go
View file
Open in desktop
Expand Up @@ -1287,6 +1287,16 @@ using the AllowUnexported option.`, "\n"),
return &b
}(): 0},
reason: "printing map keys should have some verbosity limit imposed",
}, {
label: label + "/LargeStringInInterface",
x: struct{ X interface{} }{"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam sit amet pretium ligula, at gravida quam. Integer iaculis, velit at sagittis ultricies, lacus metus scelerisque turpis, ornare feugiat nulla nisl ac erat. Maecenas elementum ultricies libero, sed efficitur lacus molestie non. Nulla ac pretium dolor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Pellentesque mi lorem, consectetur id porttitor id, sollicitudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis."},
y: struct{ X interface{} }{"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam sit amet pretium ligula, at gravida quam. Integer iaculis, velit at sagittis ultricies, lacus metus scelerisque turpis, ornare feugiat nulla nisl ac erat. Maecenas elementum ultricies libero, sed efficitur lacus molestie non. Nulla ac pretium dolor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Pellentesque mi lorem, consectetur id porttitor id, sollicitudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis,"},
reason: "strings within an interface should benefit from specialized diffing",
}, {
label: label + "/LargeBytesInInterface",
x: struct{ X interface{} }{[]byte("Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam sit amet pretium ligula, at gravida quam. Integer iaculis, velit at sagittis ultricies, lacus metus scelerisque turpis, ornare feugiat nulla nisl ac erat. Maecenas elementum ultricies libero, sed efficitur lacus molestie non. Nulla ac pretium dolor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Pellentesque mi lorem, consectetur id porttitor id, sollicitudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis.")},
y: struct{ X interface{} }{[]byte("Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam sit amet pretium ligula, at gravida quam. Integer iaculis, velit at sagittis ultricies, lacus metus scelerisque turpis, ornare feugiat nulla nisl ac erat. Maecenas elementum ultricies libero, sed efficitur lacus molestie non. Nulla ac pretium dolor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Pellentesque mi lorem, consectetur id porttitor id, sollicitudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis,")},
reason: "bytes slice within an interface should benefit from specialized diffing",
}}
}

Expand Down
25 changes: 21 additions & 4 deletions cmp/report_slices.go
View file
Open in desktop
Expand Up @@ -26,8 +26,6 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
return false // No differences detected
case !v.ValueX.IsValid() || !v.ValueY.IsValid():
return false // Both values must be valid
case v.Type.Kind() == reflect.Slice && (v.ValueX.Len() == 0 || v.ValueY.Len() == 0):
return false // Both slice values have to be non-empty
case v.NumIgnored > 0:
return false // Some ignore option was used
case v.NumTransformed > 0:
Expand All @@ -45,7 +43,16 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
return false
}

switch t := v.Type; t.Kind() {
// Check whether this is an interface with the same concrete types.
t := v.Type
vx, vy := v.ValueX, v.ValueY
if t.Kind() == reflect.Interface && !vx.IsNil() && !vy.IsNil() && vx.Elem().Type() == vy.Elem().Type() {
vx, vy = vx.Elem(), vy.Elem()
t = vx.Type()
}

// Check whether we provide specialized diffing for this type.
switch t.Kind() {
case reflect.String:
case reflect.Array, reflect.Slice:
// Only slices of primitive types have specialized handling.
Expand All @@ -57,6 +64,11 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
return false
}

// Both slice values have to be non-empty.
if t.Kind() == reflect.Slice && (vx.Len() == 0 || vy.Len() == 0) {
return false
}

// If a sufficient number of elements already differ,
// use specialized formatting even if length requirement is not met.
if v.NumDiff > v.NumSame {
Expand All @@ -68,7 +80,7 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {

// Use specialized string diffing for longer slices or strings.
const minLength = 64
return v.ValueX.Len() >= minLength && v.ValueY.Len() >= minLength
return vx.Len() >= minLength && vy.Len() >= minLength
}

// FormatDiffSlice prints a diff for the slices (or strings) represented by v.
Expand All @@ -77,6 +89,11 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode {
assert(opts.DiffMode == diffUnknown)
t, vx, vy := v.Type, v.ValueX, v.ValueY
if t.Kind() == reflect.Interface {
vx, vy = vx.Elem(), vy.Elem()
t = vx.Type()
opts = opts.WithTypeMode(emitType)
}

// Auto-detect the type of the data.
var isLinedText, isText, isBinary bool
Expand Down
22 changes: 22 additions & 0 deletions cmp/testdata/diffs
View file
Open in desktop
Expand Up @@ -1014,6 +1014,28 @@
+ &⟪0xdeadf00f⟫⟪ptr:0xdeadf00f, len:1048576, cap:1048576⟫{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, ...}: 0,
}
>>> TestDiff/Reporter/LargeMapKey
<<< TestDiff/Reporter/LargeStringInInterface
struct{ X interface{} }{
X: strings.Join({
... // 485 identical bytes
"s mus. Pellentesque mi lorem, consectetur id porttitor id, solli",
"citudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis",
- ".",
+ ",",
}, ""),
}
>>> TestDiff/Reporter/LargeStringInInterface
<<< TestDiff/Reporter/LargeBytesInInterface
struct{ X interface{} }{
X: bytes.Join({
... // 485 identical bytes
"s mus. Pellentesque mi lorem, consectetur id porttitor id, solli",
"citudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis",
- ".",
+ ",",
}, ""),
}
>>> TestDiff/Reporter/LargeBytesInInterface
<<< TestDiff/EmbeddedStruct/ParentStructA/Inequal
teststructs.ParentStructA{
privateStruct: teststructs.privateStruct{
Expand Down