nested non-addressable values cannot be merged

[bionoren]

trafficstars

nested non-addressable values cannot be merged

My specific problem is this: given two structs with interface{} fields that contain struct values of the same type, the interface{} fields are not deeply merged.

	type testStruct struct {
		Name string
		Value string
	}
	type interfaceStruct struct {
		Field interface{}
	}
	dst := interfaceStruct{
		Field: &testStruct{
			Value: "keepMe",
		},
	}

	src := interfaceStruct{
		Field: &testStruct{
			Name: "newName",
		},
	}

	mergo.Merge(&dst, src)

expected value of dst:

{
	Field: {
		Name: "newName",
		Value: "keepMe",
	},
}

actual value of dst:

{
	Field: {
		Name: "",
		Value: "keepMe",
	},
}

The reason for this is that merge.go:267 calls deepMerge with the interface values ([value].Elem()). However, since those are values in a field of type interface{}, they're not addressable (did not expect that, learned something today). The recursive call will end up in the default case of that switch statement, and on line merge.go:275, CanSet() will be false (because CanAddr() is false), and you'll try assigning dst = src.

But there's a problem with that. dst is of type reflect.Value, which is not merely a Value, it's a value. It's not a pointer. That assignment can't modify any memory outside of that method. As soon as the argument for deepMerge, dst, stops being addressable, deepMerge can no longer modify dst, which is its implicit return value. It may as well return immediately.

deepMerge used to return (dst reflect.Value, err error). It was changed recently, and I'm not sure why. The change, unfortunately, prevents me from deeply merging equal values in interfaces, as appears to be the intent of merge.go:266-271. Would you be open to going back to returning (dst, err) so that dst can be set back up at the interface level where it's still addressable? If you are ok with that change, I'm happy to make a pull request for it!

All files and line numbers as of v0.3.11

Upvote & Fund

• We're using Polar.sh so you can upvote and help fund this issue.
• We receive the funding once the issue is completed & confirmed by you.
• Thank you in advance for helping prioritize & fund our backlog.

[xscode-auto-reply[bot]]

Thanks for opening a new issue. The team has been notified and will review it as soon as possible. For urgent issues and priority support, visit https://xscode.com/imdario/mergo

[darccio]

Adding this to v2 backlog.

[dionysius]

My issue was closed in favor of this one. If it is valuable I had written a testcase https://github.com/darccio/mergo/pull/168 that could be cherry-picked. Just noting this because that PR got automatically closed as well.

[darccio]

@dionysius No new features or default behaviours will be added to v1. I'll publish in a few minutes an updated README stating this.