Skip to content

Commit dc6435e

Browse files
authored
De-virtualize interfaces for specialized diffing (#254)
Specialized diffing strings and slices should occur for interface types where both values have the same concrete type. This is especially relevant for protocmp.Transform, which transforms every proto.Message as a map[string]interface{}.
1 parent e9947a2 commit dc6435e

File tree

3 files changed

+53
-4
lines changed

3 files changed

+53
-4
lines changed

cmp/compare_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,6 +1287,16 @@ using the AllowUnexported option.`, "\n"),
12871287
return &b
12881288
}(): 0},
12891289
reason: "printing map keys should have some verbosity limit imposed",
1290+
}, {
1291+
label: label + "/LargeStringInInterface",
1292+
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."},
1293+
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,"},
1294+
reason: "strings within an interface should benefit from specialized diffing",
1295+
}, {
1296+
label: label + "/LargeBytesInInterface",
1297+
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.")},
1298+
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,")},
1299+
reason: "bytes slice within an interface should benefit from specialized diffing",
12901300
}}
12911301
}
12921302

cmp/report_slices.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
2626
return false // No differences detected
2727
case !v.ValueX.IsValid() || !v.ValueY.IsValid():
2828
return false // Both values must be valid
29-
case v.Type.Kind() == reflect.Slice && (v.ValueX.Len() == 0 || v.ValueY.Len() == 0):
30-
return false // Both slice values have to be non-empty
3129
case v.NumIgnored > 0:
3230
return false // Some ignore option was used
3331
case v.NumTransformed > 0:
@@ -45,7 +43,16 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
4543
return false
4644
}
4745

48-
switch t := v.Type; t.Kind() {
46+
// Check whether this is an interface with the same concrete types.
47+
t := v.Type
48+
vx, vy := v.ValueX, v.ValueY
49+
if t.Kind() == reflect.Interface && !vx.IsNil() && !vy.IsNil() && vx.Elem().Type() == vy.Elem().Type() {
50+
vx, vy = vx.Elem(), vy.Elem()
51+
t = vx.Type()
52+
}
53+
54+
// Check whether we provide specialized diffing for this type.
55+
switch t.Kind() {
4956
case reflect.String:
5057
case reflect.Array, reflect.Slice:
5158
// Only slices of primitive types have specialized handling.
@@ -57,6 +64,11 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
5764
return false
5865
}
5966

67+
// Both slice values have to be non-empty.
68+
if t.Kind() == reflect.Slice && (vx.Len() == 0 || vy.Len() == 0) {
69+
return false
70+
}
71+
6072
// If a sufficient number of elements already differ,
6173
// use specialized formatting even if length requirement is not met.
6274
if v.NumDiff > v.NumSame {
@@ -68,7 +80,7 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
6880

6981
// Use specialized string diffing for longer slices or strings.
7082
const minLength = 64
71-
return v.ValueX.Len() >= minLength && v.ValueY.Len() >= minLength
83+
return vx.Len() >= minLength && vy.Len() >= minLength
7284
}
7385

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

8198
// Auto-detect the type of the data.
8299
var isLinedText, isText, isBinary bool

cmp/testdata/diffs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,28 @@
10141014
+ &⟪0xdeadf00f⟫⟪ptr:0xdeadf00f, len:1048576, cap:1048576⟫{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, ...}: 0,
10151015
}
10161016
>>> TestDiff/Reporter/LargeMapKey
1017+
<<< TestDiff/Reporter/LargeStringInInterface
1018+
struct{ X interface{} }{
1019+
X: strings.Join({
1020+
... // 485 identical bytes
1021+
"s mus. Pellentesque mi lorem, consectetur id porttitor id, solli",
1022+
"citudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis",
1023+
- ".",
1024+
+ ",",
1025+
}, ""),
1026+
}
1027+
>>> TestDiff/Reporter/LargeStringInInterface
1028+
<<< TestDiff/Reporter/LargeBytesInInterface
1029+
struct{ X interface{} }{
1030+
X: bytes.Join({
1031+
... // 485 identical bytes
1032+
"s mus. Pellentesque mi lorem, consectetur id porttitor id, solli",
1033+
"citudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis",
1034+
- ".",
1035+
+ ",",
1036+
}, ""),
1037+
}
1038+
>>> TestDiff/Reporter/LargeBytesInInterface
10171039
<<< TestDiff/EmbeddedStruct/ParentStructA/Inequal
10181040
teststructs.ParentStructA{
10191041
privateStruct: teststructs.privateStruct{

0 commit comments

Comments
 (0)