Skip to content

Commit 328fea4

Browse files
author
Antoine Pelisse
committed
value.Value should basically never be a pointer
Change the code so that the value.Value should never be a pointer. Either the value is nil, in which case the underlying implementation can indicate that, or the value is not present, in which case the value.Value interface is nil. But we don't need a third level of indirection.
1 parent c1734b6 commit 328fea4

File tree

12 files changed

+55
-64
lines changed

12 files changed

+55
-64
lines changed

‎fieldpath/fromvalue.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ type objectWalker struct {
4343

4444
func (w *objectWalker) walk() {
4545
switch {
46+
case w.value.IsNull():
4647
case w.value.IsFloat():
4748
case w.value.IsInt():
4849
case w.value.IsString():
@@ -54,11 +55,11 @@ func (w *objectWalker) walk() {
5455
case w.value.IsList():
5556
// If the list were atomic, we'd break here, but we don't have
5657
// a schema, so we can't tell.
57-
58-
for i := 0; i < w.value.List().Length(); i++ {
58+
list := w.value.List()
59+
for i := 0; i < list.Length(); i++ {
5960
w2 := *w
60-
w2.path = append(w.path, GuessBestListPathElement(i, w.value.List().At(i)))
61-
w2.value = w.value.List().At(i)
61+
w2.path = append(w.path, GuessBestListPathElement(i, list.At(i)))
62+
w2.value = list.At(i)
6263
w2.walk()
6364
}
6465
return

‎fieldpath/path.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ func MakePath(parts ...interface{}) (Path, error) {
9393
return nil, fmt.Errorf("associative list key type path elements must have at least one key (got zero)")
9494
}
9595
fp = append(fp, PathElement{Key: t})
96-
case *value.Value:
96+
case value.Value:
9797
// TODO: understand schema and verify that this is a set type
9898
// TODO: make a copy of t
99-
fp = append(fp, PathElement{Value: t})
99+
fp = append(fp, PathElement{Value: &t})
100100
default:
101101
return nil, fmt.Errorf("unable to make %#v into a path element", p)
102102
}

‎fieldpath/path_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,7 @@ import (
2323
)
2424

2525
var (
26-
_V = func(i interface{}) *value.Value {
27-
v := value.NewValueInterface(i)
28-
return &v
29-
}
26+
_V = value.NewValueInterface
3027
)
3128

3229
func TestPathString(t *testing.T) {

‎merge/conflict_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,7 @@ var (
2929
_NS = fieldpath.NewSet
3030
_P = fieldpath.MakePathOrDie
3131
_KBF = fieldpath.KeyByFields
32-
_V = func(i interface{}) *value.Value {
33-
v := value.NewValueInterface(i)
34-
return &v
35-
}
32+
_V = value.NewValueInterface
3633
)
3734

3835
func TestNewFromSets(t *testing.T) {

‎typed/helpers.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ type atomHandler interface {
8585
doMap(*schema.Map) ValidationErrors
8686
}
8787

88-
func resolveSchema(s *schema.Schema, tr schema.TypeRef, v *value.Value, ah atomHandler) ValidationErrors {
88+
func resolveSchema(s *schema.Schema, tr schema.TypeRef, v value.Value, ah atomHandler) ValidationErrors {
8989
a, ok := s.Resolve(tr)
9090
if !ok {
9191
return errorf("schema error: no type found matching: %v", *tr.NamedType)
@@ -99,18 +99,18 @@ func resolveSchema(s *schema.Schema, tr schema.TypeRef, v *value.Value, ah atomH
9999
// If val is of a type allowed by atom, return a copy of atom with all other types set to nil.
100100
// if val is nil, or is not of a type allowed by atom, just return the original atom,
101101
// and validation will fail at a later stage. (with a more useful error)
102-
func deduceAtom(atom schema.Atom, val *value.Value) schema.Atom {
102+
func deduceAtom(atom schema.Atom, val value.Value) schema.Atom {
103103
switch {
104-
case val == nil, *val == nil:
105-
case (*val).IsFloat(), (*val).IsInt(), (*val).IsString(), (*val).IsBool():
104+
case val == nil:
105+
case val.IsFloat(), val.IsInt(), val.IsString(), val.IsBool():
106106
if atom.Scalar != nil {
107107
return schema.Atom{Scalar: atom.Scalar}
108108
}
109-
case (*val).IsList():
109+
case val.IsList():
110110
if atom.List != nil {
111111
return schema.Atom{List: atom.List}
112112
}
113-
case (*val).IsMap():
113+
case val.IsMap():
114114
if atom.Map != nil {
115115
return schema.Atom{Map: atom.Map}
116116
}

‎typed/merge.go

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import (
2525
)
2626

2727
type mergingWalker struct {
28-
lhs *value.Value
29-
rhs *value.Value
28+
lhs value.Value
29+
rhs value.Value
3030
schema *schema.Schema
3131
typeRef schema.TypeRef
3232

@@ -58,10 +58,10 @@ type mergeRule func(w *mergingWalker)
5858
var (
5959
ruleKeepRHS = mergeRule(func(w *mergingWalker) {
6060
if w.rhs != nil {
61-
v := (*w.rhs).Interface()
61+
v := w.rhs.Interface()
6262
w.out = &v
6363
} else if w.lhs != nil {
64-
v := (*w.lhs).Interface()
64+
v := w.lhs.Interface()
6565
w.out = &v
6666
}
6767
})
@@ -148,13 +148,13 @@ func (w *mergingWalker) finishDescent(w2 *mergingWalker) {
148148
*w.spareWalkers = append(*w.spareWalkers, w2)
149149
}
150150

151-
func (w *mergingWalker) derefMap(prefix string, v *value.Value, dest *value.Map) (errs ValidationErrors) {
151+
func (w *mergingWalker) derefMap(prefix string, v value.Value, dest *value.Map) (errs ValidationErrors) {
152152
// taking dest as input so that it can be called as a one-liner with
153153
// append.
154154
if v == nil {
155155
return nil
156156
}
157-
m, err := mapValue(*v)
157+
m, err := mapValue(v)
158158
if err != nil {
159159
return errorf("%v: %v", prefix, err)
160160
}
@@ -219,10 +219,9 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
219219
}
220220
observedLHS.Insert(pe)
221221
w2 := w.prepareDescent(pe, t.ElementType)
222-
val := value.Value(child)
223-
w2.lhs = &val
222+
w2.lhs = value.Value(child)
224223
if rchild, ok := observedRHS.Get(pe); ok {
225-
w2.rhs = &rchild
224+
w2.rhs = rchild
226225
}
227226
if newErrs := w2.merge(); len(newErrs) > 0 {
228227
errs = append(errs, newErrs.WithPrefix(pe.String())...)
@@ -239,7 +238,7 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
239238
}
240239
value, _ := observedRHS.Get(pe)
241240
w2 := w.prepareDescent(pe, t.ElementType)
242-
w2.rhs = &value
241+
w2.rhs = value
243242
if newErrs := w2.merge(); len(newErrs) > 0 {
244243
errs = append(errs, newErrs.WithPrefix(pe.String())...)
245244
} else if w2.out != nil {
@@ -256,13 +255,13 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
256255
return errs
257256
}
258257

259-
func (w *mergingWalker) derefList(prefix string, v *value.Value, dest *value.List) (errs ValidationErrors) {
258+
func (w *mergingWalker) derefList(prefix string, v value.Value, dest *value.List) (errs ValidationErrors) {
260259
// taking dest as input so that it can be called as a one-liner with
261260
// append.
262261
if v == nil {
263262
return nil
264263
}
265-
l, err := listValue(*v)
264+
l, err := listValue(v)
266265
if err != nil {
267266
return errorf("%v: %v", prefix, err)
268267
}
@@ -294,7 +293,7 @@ func (w *mergingWalker) doList(t *schema.List) (errs ValidationErrors) {
294293
return errs
295294
}
296295

297-
func (w *mergingWalker) visitMapItem(t *schema.Map, out map[string]interface{}, key string, lhs, rhs *value.Value) (errs ValidationErrors) {
296+
func (w *mergingWalker) visitMapItem(t *schema.Map, out map[string]interface{}, key string, lhs, rhs value.Value) (errs ValidationErrors) {
298297
fieldType := t.ElementType
299298
if sf, ok := t.FindField(key); ok {
300299
fieldType = sf.Type
@@ -317,14 +316,14 @@ func (w *mergingWalker) visitMapItems(t *schema.Map, lhs, rhs value.Map) (errs V
317316

318317
if lhs != nil {
319318
lhs.Iterate(func(key string, val value.Value) bool {
320-
var rval *value.Value
319+
var rval value.Value
321320
if rhs != nil {
322321
if item, ok := rhs.Get(key); ok {
323-
rval = &item
324-
defer (*rval).Recycle()
322+
rval = item
323+
defer rval.Recycle()
325324
}
326325
}
327-
errs = append(errs, w.visitMapItem(t, out, key, &val, rval)...)
326+
errs = append(errs, w.visitMapItem(t, out, key, val, rval)...)
328327
return true
329328
})
330329
}
@@ -337,7 +336,7 @@ func (w *mergingWalker) visitMapItems(t *schema.Map, lhs, rhs value.Map) (errs V
337336
return true
338337
}
339338
}
340-
errs = append(errs, w.visitMapItem(t, out, key, nil, &val)...)
339+
errs = append(errs, w.visitMapItem(t, out, key, nil, val)...)
341340
return true
342341
})
343342
}

‎typed/remove.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func removeItemsWithSchema(val value.Value, toRemove *fieldpath.Set, schema *sch
3232
schema: schema,
3333
toRemove: toRemove,
3434
}
35-
resolveSchema(schema, typeRef, &val, w)
35+
resolveSchema(schema, typeRef, val, w)
3636
return value.NewValueInterface(w.out)
3737
}
3838

‎typed/tofieldset.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (v *toFieldSetWalker) finishDescent(v2 *toFieldSetWalker) {
8282
}
8383

8484
func (v *toFieldSetWalker) toFieldSet() ValidationErrors {
85-
return resolveSchema(v.schema, v.typeRef, &v.value, v)
85+
return resolveSchema(v.schema, v.typeRef, v.value, v)
8686
}
8787

8888
func (v *toFieldSetWalker) doScalar(t *schema.Scalar) ValidationErrors {

‎typed/toset_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,7 @@ var (
4242
_NS = fieldpath.NewSet
4343
_P = fieldpath.MakePathOrDie
4444
_KBF = fieldpath.KeyByFields
45-
_V = func(i interface{}) *value.Value {
46-
v := value.NewValueInterface(i)
47-
return &v
48-
}
45+
_V = value.NewValueInterface
4946
)
5047

5148
var fieldsetCases = []fieldsetTestCase{{

‎typed/typed.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func (tv TypedValue) Compare(rhs *TypedValue) (c *Comparison, err error) {
119119
c.Added.Insert(w.path)
120120
} else if w.rhs == nil {
121121
c.Removed.Insert(w.path)
122-
} else if !value.Equals(*w.rhs, *w.lhs) {
122+
} else if !value.Equals(w.rhs, w.lhs) {
123123
// TODO: Equality is not sufficient for this.
124124
// Need to implement equality check on the value type.
125125
c.Modified.Insert(w.path)
@@ -158,7 +158,7 @@ func (tv TypedValue) NormalizeUnions(new *TypedValue) (*TypedValue, error) {
158158
var errs ValidationErrors
159159
var normalizeFn = func(w *mergingWalker) {
160160
if w.rhs != nil {
161-
v := (*w.rhs).Interface()
161+
v := w.rhs.Interface()
162162
w.out = &v
163163
}
164164
if err := normalizeUnions(w); err != nil {
@@ -184,7 +184,7 @@ func (tv TypedValue) NormalizeUnionsApply(new *TypedValue) (*TypedValue, error)
184184
var errs ValidationErrors
185185
var normalizeFn = func(w *mergingWalker) {
186186
if w.rhs != nil {
187-
v := (*w.rhs).Interface()
187+
v := w.rhs.Interface()
188188
w.out = &v
189189
}
190190
if err := normalizeUnionsApply(w); err != nil {
@@ -232,8 +232,8 @@ func merge(lhs, rhs *TypedValue, rule, postRule mergeRule) (*TypedValue, error)
232232
mwPool.Put(mw)
233233
}()
234234

235-
mw.lhs = &lhs.value
236-
mw.rhs = &rhs.value
235+
mw.lhs = lhs.value
236+
mw.rhs = rhs.value
237237
mw.schema = lhs.schema
238238
mw.typeRef = lhs.typeRef
239239
mw.rule = rule

0 commit comments

Comments
 (0)