Go: proposal: Go 2: disallow "bare" nil values when comparing to an interface

[gwd]

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:

func foo() *bar {
  // ...
  if something_wrong {
    return nil;
  }
}

var x interface{}

x = bar()

if x != nil {
  // Dereference x
}

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 (perhaps reflect.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:

if x != interface{}(nil) {
   // ...
}

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:

1. #24635
2. #22729
3. #21538
4. #27890
5. #30294

[ianlancetaylor]

Tempting, but since error is an interface type it would mean that all comparisons that look like err != nil would be rejected. I don't think that is feasible.

[gwd]

On the contrary, that's exactly why we need something like this.

The fact is, err != nil is a trap: 99.9% if the time, it's going to do exactly what you want; but if anything anywhere in the call chain accidentally returned you a nil value of some other pointer type, your err != nil check will do the wrong thing. If we're going to have a language where error(nil) != SpecificErrorType(nil) (and I don't see how Go would still be Go without it), then it is _simply not safe_ to compare an interface to nil.

We could have an is_nil() keyword or something instead, which effectively did reflect.ValueOf(display).IsNil().

[ianlancetaylor]

If we disallow err != nil, then essentially every Go package in existence will fail to compile, and essentially all existing Go documentation will be wrong. That is a non-starter.

[gwd]

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 SpecificErrorType(nil), and making your error == nil check do the wrong thing. My whole reason for using a language like Go rather than a language like Python is to be able to avoid exactly this sort of problem.

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.

[ianlancetaylor]

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 err != nil.

[gwd]

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 err != nil is already "magic" (in that the type of nil changes based on the context) is to have the compiler interpret this to be equivalent to reflect.ValueOf(err).IsNil() when comparing an interface to a bare nil keyword; and (again), to have the current behavior only when the type is specified.

[OneOfOne]

A simple counter proposal is maybe adding another keyword for nil that would deep match, something along the lines of:

var x0 interface{} = (*string)(nil)
var x1 interface{} = nil
x0 == nil // false
x0 == ifnil // true (ifnil is just an idea)
x1 == nil == ifnil

[gwd]

@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 x0 == ifnil (or, as I recommended above, is_nil(x0) (or probably IsNil(x0) since Go prefers CamelCase to snake_case)), I would probably forget most of the time and use the unsafe version, x0 == nil instead.

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 reflect.ValueOf(x).IsNil() when comparing to nil. Having a compact way to write that would certainly be helpful, but even more helpful would be a way to tell the the compiler reminded me to use it and not the other one I almost certainly didn't mean.

Or even better yet, did what I almost certainly did mean.

[dpinela]

Using reflect.ValueOf(x).IsNil() everywhere would also be a non-starter, for performance reasons if nothing else. I think the best solution to this problem is to change the rules for pointer-to-interface conversions (like #30294), or interface-to-nil comparisons. Both are breaking changes, though.

[gwd]

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 isnil() keyword that the compiler would have an optimized version, rather than calling out to the reflect package.

[go101]

I feel the fact that nil represents as zero values of several kinds of types is half-consistent, which is the source of some confusions.
I remembered there were some proposals to solve the same problem in two ways:

1. let nil be able to represent as zero values of any kinds of types. After all, nil in English means zero (I can't find back this proposal now again.).
2. let nil only represent as zero values of interface types, and use some other predeclared identifiers for pointers/slices/maps/channels/functions. (Or use another predeclared identifier to represent zero values of interface types.)

Personally, I think those are better than the current one.

[go101]

An advantage of letting nil be able to represent as zero values of any kinds of types is this will make some conveniences for future custom generics. When we want to reset a value of generic type, just assign nil to it.

[ianlancetaylor]

See #22729 (mentioned in the original issue submission).

[beoran]

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...

[ianlancetaylor]

@beoran I think you are looking for #30294.

[gwd]

@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.

[beoran]

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.

[ibrt]

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 nil case is the worst, it applies to other types as well. Consider the following example:

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 false twice. We are used to the fact that constants are automatically cast to the right type when referenced, except in this case it doesn't work as one intuitively would expect.

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.

[ianlancetaylor]

There may be something to change in this area, but this specific proposal would invalidate essentially all existing Go code, and cannot be adopted.