Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spec: disallow duplicate keys via variables in keyed constructors #37682

Open
artekzet opened this issue Mar 5, 2020 · 1 comment
Open

spec: disallow duplicate keys via variables in keyed constructors #37682

artekzet opened this issue Mar 5, 2020 · 1 comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@artekzet
Copy link

artekzet commented Mar 5, 2020

What version of Go are you using (go version)?

go version go1.13.8 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/big0ne/Library/Caches/go-build"
GOENV="/Users/big0ne/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/big0ne/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/ql/g_n75d6510s0lxhzzs70h5g40000gn/T/go-build587896349=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I have struct like this:
type person struct { first string last string skills []string }

and I have two variables in this type:

` p1 := person{
first: "John",
last: "Stallone",
skills: []string{"Java", "Go"},
}

p2 := person{
	first:  "Mark",
	last:   "Stallone",
	skills: []string{"C++", "Python", "Go"},
}`

If I wanted create something like this:

persons := map[string]person{ "Stallone": p1, "Stallone": p2, }

it would be obvious error, but if I try this:

persons := map[string]person{ p1.last: p1, p2.last: p2, }

there is no problem.

And result may be confusing, because second key overwrites previous one. For example for this code:

for _, v := range persons { fmt.Println(v.first) fmt.Println(v.last) for i, val := range v.skills { fmt.Println(i+1, val) } fmt.Println("===") }

I get only one person with skills, when I have map with two values in there.

What did you expect to see?

Consistency would be better (so we should let developer write map with two keys all the time or we should block every try of duplication by raise an exception)

What did you see instead?

Mark Stallone 1 C++ 2 Python 3 Go

@randall77
Copy link
Contributor

persons := map[string]person{ p1.last: p1, p2.last: p2, }

Matches the behavior of

persons := map[string]person{}
persons[p1.last] = p1
persons[p2.last] = p2

I hope we agree that the latter is fine and correctly does the overwrite.

I see the error generated by map[string]person{ "Stallone": p1, "Stallone": p2, } as more of the special case, where we happen to be able to tell statically that an overwrite always happens. It's not so much that it causes an overwrite, but that one of those entries is statically known to be useless. Kind of like unreachable code.

But I see your side of the argument also. If the semantics of the constructor are defined to require uniqueness, then we should check that.

We could get rid of the duplicate key error, as that's backwards compatible. It's harder to change the language to allow the constructor to panic, as that's not backwards compatible. Such a change really needs to be worth the churn, and I'm not sure that's true here.

If we go this route (make duplicates panic), we should also change the semantics of other keyed constructors (slices and arrays). But those seem harder to do, e.g.

[1000]int{x: 0, y:0, z:0, w: 1}

Suppose x == w, so we want to panic. When we're setting a[w]=1, how do we know if that the entry a[x] has already been set? We don't have "presence" bits like we do in maps.

@toothrot toothrot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 9, 2020
@toothrot toothrot added this to the Backlog milestone Mar 9, 2020
@seankhliao seankhliao changed the title It is possible to duplicate key in map if I use struct's field spec: disallow duplicate keys via variables in keyed constructors Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

3 participants