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

Jump to bottom

spec: add duplicate nil to the implementation restriction for switches #28378

Closed
griesemer opened this issue Oct 25, 2018 · 8 comments
Closed

spec: add duplicate nil to the implementation restriction for switches #28378

griesemer opened this issue Oct 25, 2018 · 8 comments
Labels
Documentation FrozenDueToAge
Milestone
Go1.19

Comments

@griesemer
Copy link
Contributor

See #28357 for details.
@griesemer griesemer added this to the Unplanned milestone Oct 25, 2018
@griesemer griesemer self-assigned this Oct 25, 2018
@griesemer griesemer mentioned this issue Oct 25, 2018
Closed
@mdempsky
Copy link
Member

mdempsky commented Dec 4, 2018

For what it's worth, cmd/compile only warns about duplicate nils for switches on non-interface expressions. That is, it doesn't warn about duplicate nil for switches on interface expressions or at all in map literals.

I'm working on a CL to unify this code. My inclination is to just stop warning about duplicate nils, rather than relax the spec to allow this. We don't even report them correctly, so I doubt anyone's triggering this in practice:

/tmp/r.go:16:2: duplicate case nil (value 0) in switch
	previous case at <unknown line number>

@gopherbot
Copy link

Change https://golang.org/cl/152544 mentions this issue: cmd/compile: unify duplicate const detection logic

@mdempsky
Copy link
Member

mdempsky commented Dec 5, 2018

Oh, actually the warnings about duplicate nil in interface switch expressions was removed just recently in eb6c433 as part of fixing #28078. And that warning did give a correct previous line number.

gopherbot pushed a commit that referenced this issue Feb 27, 2019
@mdempsky
cmd/compile: unify duplicate const detection logic
6fa7669 
Consistent logic for handling both duplicate map keys and case values,
and eliminates ad hoc value hashing code.

Also makes cmd/compile consistent with go/types's handling of
duplicate constants (see #28085), which is at least an improvement
over the status quo even if we settle on something different for the
spec.

As a side effect, this also suppresses cmd/compile's warnings about
duplicate nils in (non-interface expression) switch statements, which
was technically never allowed by the spec anyway.

Updates #28085.
Updates #28378.

Change-Id: I176a251e770c3c5bc11c2bf8d1d862db8f252a17
Reviewed-on: https://go-review.googlesource.com/c/152544
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
@griesemer griesemer modified the milestones: Unplanned, Go1.19 May 12, 2022
@griesemer
Copy link
Contributor Author

The implementation now follows exactly the spec: only duplicate integer, floating-point, and string constants are disallowed.

@ianlancetaylor
Copy link
Contributor

Note that the implementation also rejects duplicate boolean constants.

@griesemer
Copy link
Contributor Author

@ianlancetaylor That is not the case, neither with 1.17, nor 1.18.

@ianlancetaylor
Copy link
Contributor

When I run https://go.dev/play/p/BHQnDSDJK_T I see "./prog.go:6:37: duplicate key false in map literal".

@ianlancetaylor
Copy link
Contributor

Ah, sorry, you are talking about switches and I am talking about composite literals.

@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge
Projects
None yet
Milestone
Go1.19
Development

No branches or pull requests
4 participants
@mdempsky @ianlancetaylor @griesemer @gopherbot