goyaml

Merge lp:~niemeyer/goyaml/interfaces-are-immutable-values into lp:~gophers/goyaml/trunk

  1. interfaces-are-immutable-values
  2. Merge into trunk
Proposed by Gustavo Niemeyer
Status: Merged
Merged at revision: 34
Proposed branch: lp:~niemeyer/goyaml/interfaces-are-immutable-values
Merge into: lp:~gophers/goyaml/trunk
Diff against target: 97 lines (+20/-9)
3 files modified
.lbox (+1/-0)
decode.go (+5/-2)
decode_test.go (+14/-7)
To merge this branch: bzr merge lp:~niemeyer/goyaml/interfaces-are-immutable-values
Reviewer Review Type Date Requested Status
The Go Language Gophers Pending
Review via email: mp+96669@code.launchpad.net

Description of the change

Unbreak goyaml after interface mutability fix in reflect.

https://codereview.appspot.com/5784060/

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :
Download full text (3.7 KiB)

Reviewers: mp+96669_code.launchpad.net,

Message:
Please take a look.

Description:

https://code.launchpad.net/~niemeyer/goyaml/interfaces-are-immutable-values/+merge/96669

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/5784060/

Affected files:
   A .lbox
   M decode.go
   M decode_test.go

Index: .lbox
=== <email address hidden> >
<email address hidden>
=== added file '.lbox'
--- .lbox 1970-01-01 00:00:00 +0000
+++ .lbox 2012-03-08 21:51:14 +0000
@@ -0,0 +1,1 @@
+propose -cr -for=lp:goyaml

Index: decode.go
=== <email address hidden> >
<email address hidden>
=== modified file 'decode.go'
--- decode.go 2011-12-21 17:47:08 +0000
+++ decode.go 2012-03-08 21:50:15 +0000
@@ -375,11 +375,11 @@
   if set := d.setter("!!seq", &out, &good); set != nil {
    defer set()
   }
+ var iface reflect.Value
   if out.Kind() == reflect.Interface {
    // No type hints. Will have to use a generic sequence.
- iface := out
+ iface = out
    out = settableValueOf(make([]interface{}, 0))
- iface.Set(out)
   }

   if out.Kind() != reflect.Slice {
@@ -394,6 +394,9 @@
     out.Set(reflect.Append(out, e))
    }
   }
+ if iface.IsValid() {
+ iface.Set(out)
+ }
   return true
  }

Index: decode_test.go
=== <email address hidden> >
<email address hidden>
=== modified file 'decode_test.go'
--- decode_test.go 2011-12-21 17:47:08 +0000
+++ decode_test.go 2012-03-08 21:50:15 +0000
@@ -41,7 +41,7 @@
   {"fixed: 685_230.15", map[string]interface{}{"fixed": 685230.15}},
   //{"sexa: 190:20:30.15", map[string]interface{}{"sexa": 0}}, //
Unsupported
   {"neginf: -.inf", map[string]interface{}{"neginf": math.Inf(-1)}},
- {"notanum: .NaN", map[string]interface{}{"notanum": math.NaN()}},
+ //{"notanum: .NaN", map[string]interface{}{"notanum": math.NaN()}}, //
Equality of NaN fails.
   {"fixed: 685_230.15", map[string]float64{"fixed": 685230.15}},

   // Bools from spec
@@ -136,11 +136,18 @@
     value = pv.Interface()
    }
    err := goyaml.Unmarshal([]byte(item.data), value)
- c.Assert(err, IsNil, Bug("Item #%d", i))
- c.Assert(value, Equals, item.value)
+ c.Assert(err, IsNil, Commentf("Item #%d", i))
+ c.Assert(value, DeepEquals, item.value)
   }
  }

+func (s *S) TestUnmarshalNaN(c *C) {
+ value := map[string]interface{}{}
+ err := goyaml.Unmarshal([]byte("notanum: .NaN"), &value)
+ c.Assert(err, IsNil)
+ c.Assert(math.IsNaN(value["notanum"].(float64)), Equals, true)
+}
+
  var unmarshalErrorTests = []struct {
   data, error string
  }{
@@ -155,7 +162,7 @@
   for _, item := range unmarshalErrorTests {
    var value interface{}
    err := goyaml.Unmarshal([]byte(item.data), &value)
- c.Assert(err, ErrorMatches, item.error, Bug("Partial unmarshal: %#v",
value))
+ c.Assert(err, ErrorMatches, item.error, Commentf("Partial
unmarshal: %#v", value))
   }
  }

@@ -198,9 +205,9 @@
    err := goyaml.Unmarshal([]byte(item.data), obj)
    c.Assert(err, IsNil)
    c.Assert(obj.Field, NotNil,
- Bug("Pointer not initialized (%#v...

Read more...

Revision history for this message
Roger Peppe (rogpeppe) wrote :

On 2012/03/08 21:54:00, niemeyer wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/5784060/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

*** Submitted:

Unbreak goyaml after interface mutability fix in reflect.

R=rog
CC=
https://codereview.appspot.com/5784060

https://codereview.appspot.com/5784060/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file '.lbox'
2--- .lbox 1970-01-01 00:00:00 +0000
3+++ .lbox 2012-03-08 21:54:17 +0000
4@@ -0,0 +1,1 @@
5+propose -cr -for=lp:goyaml
6
7=== modified file 'decode.go'
8--- decode.go 2011-12-21 17:47:08 +0000
9+++ decode.go 2012-03-08 21:54:17 +0000
10@@ -375,11 +375,11 @@
11 if set := d.setter("!!seq", &out, &good); set != nil {
12 defer set()
13 }
14+ var iface reflect.Value
15 if out.Kind() == reflect.Interface {
16 // No type hints. Will have to use a generic sequence.
17- iface := out
18+ iface = out
19 out = settableValueOf(make([]interface{}, 0))
20- iface.Set(out)
21 }
22
23 if out.Kind() != reflect.Slice {
24@@ -394,6 +394,9 @@
25 out.Set(reflect.Append(out, e))
26 }
27 }
28+ if iface.IsValid() {
29+ iface.Set(out)
30+ }
31 return true
32 }
33
34
35=== modified file 'decode_test.go'
36--- decode_test.go 2011-12-21 17:47:08 +0000
37+++ decode_test.go 2012-03-08 21:54:17 +0000
38@@ -41,7 +41,7 @@
39 {"fixed: 685_230.15", map[string]interface{}{"fixed": 685230.15}},
40 //{"sexa: 190:20:30.15", map[string]interface{}{"sexa": 0}}, // Unsupported
41 {"neginf: -.inf", map[string]interface{}{"neginf": math.Inf(-1)}},
42- {"notanum: .NaN", map[string]interface{}{"notanum": math.NaN()}},
43+ //{"notanum: .NaN", map[string]interface{}{"notanum": math.NaN()}}, // Equality of NaN fails.
44 {"fixed: 685_230.15", map[string]float64{"fixed": 685230.15}},
45
46 // Bools from spec
47@@ -136,11 +136,18 @@
48 value = pv.Interface()
49 }
50 err := goyaml.Unmarshal([]byte(item.data), value)
51- c.Assert(err, IsNil, Bug("Item #%d", i))
52- c.Assert(value, Equals, item.value)
53+ c.Assert(err, IsNil, Commentf("Item #%d", i))
54+ c.Assert(value, DeepEquals, item.value)
55 }
56 }
57
58+func (s *S) TestUnmarshalNaN(c *C) {
59+ value := map[string]interface{}{}
60+ err := goyaml.Unmarshal([]byte("notanum: .NaN"), &value)
61+ c.Assert(err, IsNil)
62+ c.Assert(math.IsNaN(value["notanum"].(float64)), Equals, true)
63+}
64+
65 var unmarshalErrorTests = []struct {
66 data, error string
67 }{
68@@ -155,7 +162,7 @@
69 for _, item := range unmarshalErrorTests {
70 var value interface{}
71 err := goyaml.Unmarshal([]byte(item.data), &value)
72- c.Assert(err, ErrorMatches, item.error, Bug("Partial unmarshal: %#v", value))
73+ c.Assert(err, ErrorMatches, item.error, Commentf("Partial unmarshal: %#v", value))
74 }
75 }
76
77@@ -198,9 +205,9 @@
78 err := goyaml.Unmarshal([]byte(item.data), obj)
79 c.Assert(err, IsNil)
80 c.Assert(obj.Field, NotNil,
81- Bug("Pointer not initialized (%#v)", item.value))
82+ Commentf("Pointer not initialized (%#v)", item.value))
83 c.Assert(obj.Field.tag, Equals, item.tag)
84- c.Assert(obj.Field.value, Equals, item.value)
85+ c.Assert(obj.Field.value, DeepEquals, item.value)
86 }
87 }
88
89@@ -211,7 +218,7 @@
90 c.Assert(obj.tag, Equals, setterTests[0].tag)
91 value, ok := obj.value.(map[interface{}]interface{})
92 c.Assert(ok, Equals, true)
93- c.Assert(value["_"], Equals, setterTests[0].value)
94+ c.Assert(value["_"], DeepEquals, setterTests[0].value)
95 }
96
97 func (s *S) TestUnmarshalWithFalseSetterIgnoresValue(c *C) {

Subscribers

People subscribed via source and target branches