-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: Go 2: disallow "bare" nil values when comparing to an interface #30865
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
Comments
Tempting, but since |
On the contrary, that's exactly why we need something like this. The fact is, We could have an |
If we disallow |
But would you agree, though, that every Go package in existence currently behaves dangerously in this regard, and all existing Go documentation recommends dangerous behavior? There is nothing to stop some function way deep in the callstack -- perhaps in a package imported from a package imported from a package you import -- returning If you agree there's a problem, then we can try to come up with a practical approach to making this sort of a change more feasible. |
I'm not sure I agree that the problem is precisely as you've stated it, but I agree that there is a problem in this general area, and you've referenced earlier issues that try to address it. But at this point in the language's evolution, I don't think that we can disallow |
Well, one approach, for instance, would be to begin by allowing individual packages / files to "opt-in" to the new behavior. If the core Go people then promoted enabling this option, and changed as much documentation as possible, it might receive significant uptake; if it did then we could consider enabling the feature by default (and allowing individual files to disable it). The other approach would be, since |
A simple counter proposal is maybe adding another keyword for
|
@OneOfOne The problem with what you recommend is that it doesn't actually close the "trap": that the normal, natural thing to do -- the thing that, as @ianlancetaylor points out, basically every Go package in existence does and all existing Go documentation recommends -- works 99% of the time and does the wrong thing the other 1%. If we introduced The point of a statically typed language, to me, is to reduce the number of random things I have to remember to get right. I plan, in the future, of always using Or even better yet, did what I almost certainly did mean. |
Using |
Well yes, I don't think this can be fixed without breaking changes; that's why I'm aiming for Go 2. I was expecting that if we introduced a special |
I feel the fact that
Personally, I think those are better than the current one. |
An advantage of letting |
See #22729 (mentioned in the original issue submission). |
I think the problem is not in the comparison, but in how it's too easy to accidentally make a non-nil interface value with a nil inside of it. In fact, is there even any use to such a value? Perhaps making such a value could panic unless forced not to by a ,ok = form... |
@beoran Well I did recently see this interesting suggestion: type Config struct {
path string
}
func (c *Config) Path() string {
if c == nil {
return "/usr/home"
}
return c.path
}
func main() {
var c1 *Config
var c2 = &Config{
path: "/export",
}
fmt.Println(c1.Path(), c2.Path())
} On the whole, though, I think that's a bit too "clever" for me. |
Well, I admit that there will probably be some existing code out there that is that clever. After all, in Go, you can make methods nil-proof, it's just that most people don't bother. But yes, I prefer #30294 to this proposal, which should make it harder to accidentally wrap nil pointers. |
I have definitely been bitten by this issue before and would like to see it fixed. To me it is a type inference "bug", and while the package main
import (
"fmt"
)
func main() {
var v interface{}
v = someInt()
fmt.Println(v == 1)
v = someIntPtr()
fmt.Println(v == nil)
}
func someInt() int64 {
return 1
}
func someIntPtr() *int {
return nil
} This will print I suspect the best fix would be to make it just work with support from the runtime, but I'm not sure whether it's feasible. |
There may be something to change in this area, but this specific proposal would invalidate essentially all existing Go code, and cannot be adopted. |
Summary
Don't allow a 'bare'
nil
keyword on either side of a comparison expression (==
or!=
) if the other side is an interface; only allow comparison to a type with an explicit cast; for example,interface{}(nil)
.Motivation
The following is an ever-present trap:
The code is very obviously trying to see if x can be dereferenced; but instead it checks to see if x is some arcane type nobody uses.
Proposal
Don't allow a 'bare'
nil
keyword on either side of a comparison expression (==
or!=
). The code above would thus throw a compiler error, alerting the developer to the fact that she should do something else instead (perhapsreflect.ValueOf(x).IsNil()
or some variant thereof).If someone really did want to compare to an actual nil interface, they could use an explicit cast, as below:
This has the advantage of changing the language as little as possible, while preventing developers from walking into one of Go's few "language traps".
References
Other proposals which try to address this topic:
typednil
keyword for checking whether an interface value is a typed nil. #24635The text was updated successfully, but these errors were encountered: