code review 7383052: encoding/json: Unmarshal into existing map elements

encoding/json: Unmarshal into existing map elements

This makes unmarshalling into a map with keys matching keys from the JSON
object use the concrete type for the respective map elements.

Fixes issue 4900.

[titanous, 2013-02-24 21:02:00]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[remyoudompheng, 2013-02-24 23:23:13]

This patch proposes an API change. People that used to expect the old behaviour
will be broken in subtle ways, so I don't think this is compatible with the Go 1
contract.

Also, your CL description uses three times the word "keys" with varying meaning
so I'm not sure I understand what behaviour you want.

[titanous, 2013-02-24 23:38:52]

On 2013/02/24 23:23:13, remyoudompheng wrote:
> This patch proposes an API change. People that used to expect the old
behaviour
> will be broken in subtle ways, so I don't think this is compatible with the Go
1
> contract.

The json.Unmarshal documentation states:

  To unmarshal JSON into an interface value, Unmarshal unmarshals the JSON into
the concrete value contained in the interface value.

If I understand this sentence correctly, the previous implementation has a bug
in it, because the concrete values of existing map elements are not used. If
this is the case, are we stuck with the bug?

> Also, your CL description uses three times the word "keys" with varying
meaning
> so I'm not sure I understand what behaviour you want.

I've clarified it a bit, I think the test makes the behavior I want quite clear.
There is also some discussion at
https://groups.google.com/forum/#!topic/golang-nuts/FZCgnNW301E

[kevlar, 2013-02-25 04:26:43]

On 2013/02/24 23:38:52, titanous wrote:
> On 2013/02/24 23:23:13, remyoudompheng wrote:
> > This patch proposes an API change. People that used to expect the old
> behaviour
> > will be broken in subtle ways, so I don't think this is compatible with the
Go
> 1
> > contract.
> 
> The json.Unmarshal documentation states:
> 
>   To unmarshal JSON into an interface value, Unmarshal unmarshals the JSON
into
> the concrete value contained in the interface value.
> 
> If I understand this sentence correctly, the previous implementation has a bug
> in it, because the concrete values of existing map elements are not used. If
> this is the case, are we stuck with the bug?

For what my opinion is worth, I believe this is a bug :).  I have a vague
recollection that it worked like this in the past, so it might even be a
regression.

> > Also, your CL description uses three times the word "keys" with varying
> meaning
> > so I'm not sure I understand what behaviour you want.
> 
> I've clarified it a bit, I think the test makes the behavior I want quite
clear.
> There is also some discussion at
> https://groups.google.com/forum/#%21topic/golang-nuts/FZCgnNW301E

[rsc, 2013-03-19 19:09:51]

I'm sorry but I can't convince myself that these are the semantics we want.
I'd rather live with the Go 1.0 bug than introduce a new Go 1.1 bug. Let's
leave this for after Go 1.1.

Thanks.
Russ

[titanous, 2013-03-26 14:42:33]

*** Abandoned ***