6 ms·

Back when we thought "Go 2" was going to have lots of breaking changes, I proposed making the comparison to an interface to nil illegal [1]. It was more or les

by gwd 6y ago

Back when we thought "Go 2" was going to have lots of breaking changes, I proposed making the comparison to an interface to nil illegal [1]. It was more or less shot down, since `error` is an interface, and basically every package library would need to be rewritten. [1] https://github.com/golang/go/issues/30865 https://github.com/golang/go/issues/30865

saturn_vk 6y ago
They weren't wrong. You might as well start an entirely new language for that change, even though it looks so small.
christophilus 6y ago
Well, instead, they could add a new mechanism that unifies nil checks while allowing the old mechanisms to continue working. Maybe generics will allow us to write a single “IsNil” function, though, which will be good enough for me.
- jerf 6y ago
  They can't, because the two nils are distinct. I hypothesize, but can not prove, that the root problem isn't that there are two nils. The root problem is that people incorrectly think nil is always invalid. It is not. This is perfectly legal (https://play.golang.org/p/71BNAyXIzRv https://play.golang.org/p/71BNAyXIzRv ) type Thing string func (t *Thing) DoSomething() { if t == nil { fmt.Println("nil thing") return } fmt.Println(string(*t)) } Nil pointers are perfectly legal values. You can call methods on them no problem. I have places where I use them; for instance, I have a memory pooling implementation where the nil pointer simply fills requests via "make" directly and does no pooling, so you can swap it out easily to test if it is causing some other bug. So I think people think of there as being "two equally invalid values", but it's not true. There's the nil interface, which is pretty useless, but an interface containing a nil pointer in it is legal and may very well completely successfully implement the interface. Checking to see whether the interface "contains" a nil pointer is almost always an error. It is not something you should ever do when you have an interface value. The real error occurs when you put the nil pointer in an interface value that can't correctly implement the interface, which is also nothing particularly special; putting anything in an interface value that can't implement the interface is an Already Lost situation. That the error then propagates around is bad, but the error occurred earlier and it's already too late to fix it by the time some other bit of code receives this broken value. Good code never has any reason to penetrate the interface and interrogate the underlying type for whether or not it is nil; if you are routinely encountering this problem, if you stop putting invalid implementations in an interface value, the problem will go away. I use all these features routinely and don't encounter this problem, because I know to never lie by putting something in an interface value that doesn't implement the interface.
  - gwd 6y ago
    > Checking to see whether the interface "contains" a nil pointer is almost always an error. It is not something you should ever do when you have an interface value. So here's where I do that: https://github.com/gwd/session-scheduler/blob/master/handle_discussion.go#L152 https://github.com/gwd/session-scheduler/blob/master/handle_... The pattern is basically: var foo = interface{} switch (something) { case A: foo = Foo() case B: foo = Bar() } Foo and Bar both return different concrete types which are nil if the request can't be handled. I want to do something if either Foo or Bar return 'nil', and also if neither case A nor case B are taken. So I've got: if foo == nil || reflect.ValueOf(foo)IsNil() { // handle the error } // Pass foo elsewhere I mean yeah, I certainly could arrange things differently; but in general I think that's not an unreasonable thing to expect to be able to do. > The real error occurs when you put the nil pointer in an interface value that can't correctly implement the interface... putting anything in an interface value that can't implement the interface is an Already Lost situation. First of all, that's not the case in my example above: nil can satisfy every method of `interface{}`. Secondly, my compiler refuses to compile if I assign something other than a nil pointer that can't implement the interface. If putting nil into an interface is such a bad idea, the compiler should prevent you from doing it.
    - jerf 6y ago
      "First of all, that's not the case in my example above: nil can satisfy every method of `interface{}`." Yes, that's true; in this case the lie is elsewhere. You said "interface{}", but you have a contract you expect the value to be able to keep. It's a contract the type system can't enforce, true. But it's a contract you're setting later in your RenderTemplate call, implicitly, and you're breaking it. The error occurs on the lines where you unconditionally set the "display" value to this contract-breaking value without checking whether it fulfills the contract you expect of it later. (I have a number of places in my code where I document something like "this function takes an interface{} but this value is expected to be able to be passed to encoding/json to be turned into JSON without error". Yes, the type system can't enforce that and it would be nice if it could. Nevertheless, if someone else calls that function and passes it something with a channel in it or something, the bug isn't in the code using the interface{}, it's in the code that passed it something that broke the contract.) You need to not slam a contract-breaking value straight into the "display" variable without checking it. The simplest thing is using an intermediate value, which will be typed: // above the switch notFound := false // at the display line userDisplay := UserGetDisplay(user, cur, true) if userDisplay == nil { notFound = true } else { display = userDisplay } // use the notFound value here to decide to display the not found page. The probably-better thing over all is to use polymorphism instead of a switch statement. I don't have a great Go-specific link, but here's a simple example: https://www.codementor.io/@uditrastogi/replace-conditional-statements-if-else-or-switch-with-polymorphism-ryl8mx4ns https://www.codementor.io/@uditrastogi/replace-conditional-s... I'd write a method that can return a notFound status or something instead, or an error I can turn into "not found", and never try to write any code that splits between different types with interface{}. It'll probably make you happier in the long run if you learn to do that. I find it a very common pattern in HTTP handlers, because it's very common for the HTTP interface to simply not directly map to an object hierarchy cleanly. "I mean yeah, I certainly could arrange things differently; but in general I think that's not an unreasonable thing to expect to be able to do." Most languages pride themselves on handing you a ton of tools and telling you to do whatever you want. You then don't have to be an expert in each individual tool, just know how to use the toolbox to get the whole job done. Go hands you a smaller toolbox, and expects you to be an expert in each one. In my experience, it can generally get the job done just fine, but you need to be ready to use each tool very well. I say this without sarcasm or harsh intent: If you're looking at my suggestions and you're saying "yeah, but I don't want to write that way", you're going to be fighting Go forever. There are in fact good ways to write that sort of code in Go. I do all the time; I don't think I have a single instance of the pattern you're using here in my entire codebase. But you need to lean in to the tools and let them guide you, not try to force them to do what you want. If you don't want to do that... and that's is perfectly valid, I'm not saying that's bad, but if you don't... you should probably stop using Go, to save your own sanity. Also... to be clear... this is all a software engineering discussion moreso than a Go defense. It is a good idea in all languages to keep track of contracts and be sure not to put things in the variables that can't fulfill the contract. It is never fun to break a contract in some bit of code, and then try to pick up the pieces later. While Go's type system is certainly far from the strongest, there aren't any languages short of the dependently-typed languages that can express all the contracts of interest, if even they can. This is not a good pattern anywhere, you just have fewer tools to hack around it in Go.
komuW 6y ago
And I agree with the proposal committee for declining your request. For a language that prides in having great backward compatibility, what you were proposing wouldn't fly.
- rkangel 6y ago
  The author did say "Back when we thought "Go 2" was going to have lots of breaking changes". It seems reasonable in that light.
  - gwd 6y ago
    Exactly; if we're going to "rip the bandaid off", just do it all at once. The idea seemed in line with other aspects of golang: for instance, you can't compare an int32 and an int64 directly -- the compiler will throw a warning if you don't cast the int32 to int64 before comparing. This is obviously because, as old-school C programmers, they recognized that automatic promotion is a trap that caused all kinds of issues. I'd argue that the "nil interface is not nil" trap is quite a bit worse. As it turns out, a lot of the improvements they were thinking about could be done without breaking backwards compatibility; which of course has some huge advantages.

by gwd 6y ago

saturn_vk 6y ago

They weren't wrong. You might as well start an entirely new language for that change, even though it looks so small.

christophilus 6y ago

Well, instead, they could add a new mechanism that unifies nil checks while allowing the old mechanisms to continue working. Maybe generics will allow us to write a single “IsNil” function, though, which will be good enough for me.

jerf 6y ago
They can't, because the two nils are distinct. I hypothesize, but can not prove, that the root problem isn't that there are two nils. The root problem is that people incorrectly think nil is always invalid. It is not. This is perfectly legal (https://play.golang.org/p/71BNAyXIzRv https://play.golang.org/p/71BNAyXIzRv ) type Thing string func (t *Thing) DoSomething() { if t == nil { fmt.Println("nil thing") return } fmt.Println(string(*t)) } Nil pointers are perfectly legal values. You can call methods on them no problem. I have places where I use them; for instance, I have a memory pooling implementation where the nil pointer simply fills requests via "make" directly and does no pooling, so you can swap it out easily to test if it is causing some other bug. So I think people think of there as being "two equally invalid values", but it's not true. There's the nil interface, which is pretty useless, but an interface containing a nil pointer in it is legal and may very well completely successfully implement the interface. Checking to see whether the interface "contains" a nil pointer is almost always an error. It is not something you should ever do when you have an interface value. The real error occurs when you put the nil pointer in an interface value that can't correctly implement the interface, which is also nothing particularly special; putting anything in an interface value that can't implement the interface is an Already Lost situation. That the error then propagates around is bad, but the error occurred earlier and it's already too late to fix it by the time some other bit of code receives this broken value. Good code never has any reason to penetrate the interface and interrogate the underlying type for whether or not it is nil; if you are routinely encountering this problem, if you stop putting invalid implementations in an interface value, the problem will go away. I use all these features routinely and don't encounter this problem, because I know to never lie by putting something in an interface value that doesn't implement the interface.
- gwd 6y ago
  > Checking to see whether the interface "contains" a nil pointer is almost always an error. It is not something you should ever do when you have an interface value. So here's where I do that: https://github.com/gwd/session-scheduler/blob/master/handle_discussion.go#L152 https://github.com/gwd/session-scheduler/blob/master/handle_... The pattern is basically: var foo = interface{} switch (something) { case A: foo = Foo() case B: foo = Bar() } Foo and Bar both return different concrete types which are nil if the request can't be handled. I want to do something if either Foo or Bar return 'nil', and also if neither case A nor case B are taken. So I've got: if foo == nil || reflect.ValueOf(foo)IsNil() { // handle the error } // Pass foo elsewhere I mean yeah, I certainly could arrange things differently; but in general I think that's not an unreasonable thing to expect to be able to do. > The real error occurs when you put the nil pointer in an interface value that can't correctly implement the interface... putting anything in an interface value that can't implement the interface is an Already Lost situation. First of all, that's not the case in my example above: nil can satisfy every method of `interface{}`. Secondly, my compiler refuses to compile if I assign something other than a nil pointer that can't implement the interface. If putting nil into an interface is such a bad idea, the compiler should prevent you from doing it.
  - jerf 6y ago
    "First of all, that's not the case in my example above: nil can satisfy every method of `interface{}`." Yes, that's true; in this case the lie is elsewhere. You said "interface{}", but you have a contract you expect the value to be able to keep. It's a contract the type system can't enforce, true. But it's a contract you're setting later in your RenderTemplate call, implicitly, and you're breaking it. The error occurs on the lines where you unconditionally set the "display" value to this contract-breaking value without checking whether it fulfills the contract you expect of it later. (I have a number of places in my code where I document something like "this function takes an interface{} but this value is expected to be able to be passed to encoding/json to be turned into JSON without error". Yes, the type system can't enforce that and it would be nice if it could. Nevertheless, if someone else calls that function and passes it something with a channel in it or something, the bug isn't in the code using the interface{}, it's in the code that passed it something that broke the contract.) You need to not slam a contract-breaking value straight into the "display" variable without checking it. The simplest thing is using an intermediate value, which will be typed: // above the switch notFound := false // at the display line userDisplay := UserGetDisplay(user, cur, true) if userDisplay == nil { notFound = true } else { display = userDisplay } // use the notFound value here to decide to display the not found page. The probably-better thing over all is to use polymorphism instead of a switch statement. I don't have a great Go-specific link, but here's a simple example: https://www.codementor.io/@uditrastogi/replace-conditional-statements-if-else-or-switch-with-polymorphism-ryl8mx4ns https://www.codementor.io/@uditrastogi/replace-conditional-s... I'd write a method that can return a notFound status or something instead, or an error I can turn into "not found", and never try to write any code that splits between different types with interface{}. It'll probably make you happier in the long run if you learn to do that. I find it a very common pattern in HTTP handlers, because it's very common for the HTTP interface to simply not directly map to an object hierarchy cleanly. "I mean yeah, I certainly could arrange things differently; but in general I think that's not an unreasonable thing to expect to be able to do." Most languages pride themselves on handing you a ton of tools and telling you to do whatever you want. You then don't have to be an expert in each individual tool, just know how to use the toolbox to get the whole job done. Go hands you a smaller toolbox, and expects you to be an expert in each one. In my experience, it can generally get the job done just fine, but you need to be ready to use each tool very well. I say this without sarcasm or harsh intent: If you're looking at my suggestions and you're saying "yeah, but I don't want to write that way", you're going to be fighting Go forever. There are in fact good ways to write that sort of code in Go. I do all the time; I don't think I have a single instance of the pattern you're using here in my entire codebase. But you need to lean in to the tools and let them guide you, not try to force them to do what you want. If you don't want to do that... and that's is perfectly valid, I'm not saying that's bad, but if you don't... you should probably stop using Go, to save your own sanity. Also... to be clear... this is all a software engineering discussion moreso than a Go defense. It is a good idea in all languages to keep track of contracts and be sure not to put things in the variables that can't fulfill the contract. It is never fun to break a contract in some bit of code, and then try to pick up the pieces later. While Go's type system is certainly far from the strongest, there aren't any languages short of the dependently-typed languages that can express all the contracts of interest, if even they can. This is not a good pattern anywhere, you just have fewer tools to hack around it in Go.

komuW 6y ago

And I agree with the proposal committee for declining your request. For a language that prides in having great backward compatibility, what you were proposing wouldn't fly.

rkangel 6y ago
The author did say "Back when we thought "Go 2" was going to have lots of breaking changes". It seems reasonable in that light.
- gwd 6y ago
  Exactly; if we're going to "rip the bandaid off", just do it all at once. The idea seemed in line with other aspects of golang: for instance, you can't compare an int32 and an int64 directly -- the compiler will throw a warning if you don't cast the int32 to int64 before comparing. This is obviously because, as old-school C programmers, they recognized that automatic promotion is a trap that caused all kinds of issues. I'd argue that the "nil interface is not nil" trap is quite a bit worse. As it turns out, a lot of the improvements they were thinking about could be done without breaking backwards compatibility; which of course has some huge advantages.