Decode: don't crash on embedded nil pointers (#808)

Also has the perks of reducing the overhead of FindByIndex: ``` name old time/op new time/op delta UnmarshalDataset/config-32 17.0ms ± 1% 17.0ms ± 1% ~ (p=1.000 n=5+5) UnmarshalDataset/canada-32 71.6ms ± 1% 71.4ms ± 1% ~ (p=1.000 n=5+5) UnmarshalDataset/citm_catalog-32 24.2ms ± 3% 23.5ms ± 2% -3.03% (p=0.032 n=5+5) UnmarshalDataset/twitter-32 9.37ms ± 1% 9.09ms ± 2% -2.97% (p=0.032 n=5+5) UnmarshalDataset/code-32 75.4ms ± 2% 74.9ms ± 0% ~ (p=0.222 n=5+5) UnmarshalDataset/example-32 147µs ±10% 136µs ± 1% -7.14% (p=0.008 n=5+5) Unmarshal/SimpleDocument/struct-32 512ns ± 2% 500ns ± 0% -2.35% (p=0.008 n=5+5) Unmarshal/SimpleDocument/map-32 721ns ± 2% 702ns ± 1% -2.68% (p=0.008 n=5+5) Unmarshal/ReferenceFile/struct-32 40.1µs ± 0% 39.6µs ± 0% -1.30% (p=0.008 n=5+5) Unmarshal/ReferenceFile/map-32 62.3µs ± 1% 60.6µs ± 0% -2.83% (p=0.008 n=5+5) Unmarshal/HugoFrontMatter-32 10.8µs ± 1% 10.5µs ± 1% -2.86% (p=0.008 n=5+5) name old speed new speed delta UnmarshalDataset/config-32 61.8MB/s ± 1% 61.8MB/s ± 1% ~ (p=1.000 n=5+5) UnmarshalDataset/canada-32 30.8MB/s ± 1% 30.8MB/s ± 1% ~ (p=1.000 n=5+5) UnmarshalDataset/citm_catalog-32 23.0MB/s ± 3% 23.8MB/s ± 2% +3.09% (p=0.032 n=5+5) UnmarshalDataset/twitter-32 47.2MB/s ± 1% 48.6MB/s ± 2% +3.09% (p=0.032 n=5+5) UnmarshalDataset/code-32 35.6MB/s ± 2% 35.9MB/s ± 0% ~ (p=0.222 n=5+5) UnmarshalDataset/example-32 55.3MB/s ±10% 59.4MB/s ± 1% +7.36% (p=0.008 n=5+5) Unmarshal/SimpleDocument/struct-32 21.5MB/s ± 2% 22.0MB/s ± 0% +2.41% (p=0.008 n=5+5) Unmarshal/SimpleDocument/map-32 15.2MB/s ± 2% 15.7MB/s ± 1% +2.74% (p=0.008 n=5+5) Unmarshal/ReferenceFile/struct-32 131MB/s ± 0% 132MB/s ± 0% +1.31% (p=0.008 n=5+5) Unmarshal/ReferenceFile/map-32 84.1MB/s ± 1% 86.6MB/s ± 0% +2.91% (p=0.008 n=5+5) Unmarshal/HugoFrontMatter-32 50.6MB/s ± 1% 52.1MB/s ± 1% +2.93% (p=0.008 n=5+5) name old alloc/op new alloc/op delta UnmarshalDataset/config-32 5.86MB ± 0% 5.86MB ± 0% ~ (p=0.579 n=5+5) UnmarshalDataset/canada-32 83.0MB ± 0% 83.0MB ± 0% ~ (p=0.651 n=5+5) UnmarshalDataset/citm_catalog-32 34.7MB ± 0% 34.7MB ± 0% ~ (p=0.548 n=5+5) UnmarshalDataset/twitter-32 12.7MB ± 0% 12.7MB ± 0% ~ (p=1.000 n=5+5) UnmarshalDataset/code-32 22.2MB ± 0% 22.2MB ± 0% ~ (p=0.841 n=5+5) UnmarshalDataset/example-32 186kB ± 0% 186kB ± 0% ~ (p=0.111 n=5+5) Unmarshal/SimpleDocument/struct-32 805B ± 0% 805B ± 0% ~ (all equal) Unmarshal/SimpleDocument/map-32 1.13kB ± 0% 1.13kB ± 0% ~ (all equal) Unmarshal/ReferenceFile/struct-32 20.9kB ± 0% 20.9kB ± 0% ~ (p=0.643 n=5+5) Unmarshal/ReferenceFile/map-32 38.3kB ± 0% 38.3kB ± 0% ~ (p=0.397 n=5+5) Unmarshal/HugoFrontMatter-32 7.44kB ± 0% 7.44kB ± 0% ~ (all equal) name old allocs/op new allocs/op delta UnmarshalDataset/config-32 227k ± 0% 227k ± 0% ~ (p=1.000 n=5+5) UnmarshalDataset/canada-32 782k ± 0% 782k ± 0% ~ (all equal) UnmarshalDataset/citm_catalog-32 192k ± 0% 192k ± 0% ~ (p=0.968 n=4+5) UnmarshalDataset/twitter-32 56.9k ± 0% 56.9k ± 0% ~ (p=0.429 n=4+5) UnmarshalDataset/code-32 1.05M ± 0% 1.05M ± 0% ~ (p=0.556 n=4+5) UnmarshalDataset/example-32 1.36k ± 0% 1.36k ± 0% ~ (all equal) Unmarshal/SimpleDocument/struct-32 9.00 ± 0% 9.00 ± 0% ~ (all equal) Unmarshal/SimpleDocument/map-32 13.0 ± 0% 13.0 ± 0% ~ (all equal) Unmarshal/ReferenceFile/struct-32 183 ± 0% 183 ± 0% ~ (all equal) Unmarshal/ReferenceFile/map-32 642 ± 0% 642 ± 0% ~ (all equal) Unmarshal/HugoFrontMatter-32 141 ± 0% 141 ± 0% ~ (all equal) ``` Fixes #807
pelletier · Aug 21, 2022 · e46d245 · e46d245
1 parent 7baa23f
commit e46d245
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 4 deletions.
diff --git a/unmarshaler.go b/unmarshaler.go
@@ -123,7 +123,7 @@ type decoder struct {
 	stashedExpr bool
 
 	// Skip expressions until a table is found. This is set to true when a
-	// table could not be create (missing field in map), so all KV expressions
+	// table could not be created (missing field in map), so all KV expressions
 	// need to be skipped.
 	skipUntilTable bool
 
@@ -483,7 +483,7 @@ func (d *decoder) handleKeyPart(key ast.Iterator, v reflect.Value, nextFn handle
 		d.errorContext.Struct = t
 		d.errorContext.Field = path
 
-		f := v.FieldByIndex(path)
+		f := fieldByIndex(v, path)
 		x, err := nextFn(key, f)
 		if err != nil || d.skipUntilTable {
 			return reflect.Value{}, err
@@ -1071,7 +1071,7 @@ func (d *decoder) handleKeyValuePart(key ast.Iterator, value *ast.Node, v reflec
 		d.errorContext.Struct = t
 		d.errorContext.Field = path
 
-		f := v.FieldByIndex(path)
+		f := fieldByIndex(v, path)
 		x, err := d.handleKeyValueInner(key, value, f)
 		if err != nil {
 			return reflect.Value{}, err
@@ -1135,6 +1135,21 @@ func initAndDereferencePointer(v reflect.Value) reflect.Value {
 	return elem
 }
 
+// Same as reflect.Value.FieldByIndex, but creates pointers if needed.
+func fieldByIndex(v reflect.Value, path []int) reflect.Value {
+	for i, x := range path {
+		v = v.Field(x)
+
+		if i < len(path)-1 && v.Kind() == reflect.Pointer {
+			if v.IsNil() {
+				v.Set(reflect.New(v.Type().Elem()))
+			}
+			v = v.Elem()
+		}
+	}
+	return v
+}
+
 type fieldPathsMap = map[string][]int
 
 var globalFieldPathsCache atomic.Value // map[danger.TypeID]fieldPathsMap
@@ -1192,7 +1207,11 @@ func forEachField(t reflect.Type, path []int, do func(name string, path []int))
 		}
 
 		if f.Anonymous && name == "" {
-			forEachField(f.Type, fieldPath, do)
+			t2 := f.Type
+			if t2.Kind() == reflect.Pointer {
+				t2 = t2.Elem()
+			}
+			forEachField(t2, fieldPath, do)
 			continue
 		}
 

diff --git a/unmarshaler_test.go b/unmarshaler_test.go
@@ -2451,6 +2451,21 @@ answer = 42
 	require.Error(t, err)
 }
 
+func TestIssue807(t *testing.T) {
+	type A struct {
+		Name string `toml:"name"`
+	}
+
+	type M struct {
+		*A
+	}
+
+	var m M
+	err := toml.Unmarshal([]byte(`name = 'foo'`), &m)
+	require.NoError(t, err)
+	require.Equal(t, "foo", m.Name)
+}
+
 func TestUnmarshalDecodeErrors(t *testing.T) {
 	examples := []struct {
 		desc string