func returnsNilError() (int, *MyErr) {
      return 0, nil
    }
    // case 1: no other use of error, nil works
    {
      i, err := returnsNilError()
      _ = i
      fmt.Printf("err is nil: %v\n", err == nil)
    }
    // case 2: someone adds a couple lines of code before that call...
    {
      _, err := os.Open("/tmp/file")
      i, err := returnsNilError()
      _ = i
      fmt.Printf("err is nil: %v\n", err == nil)
    }
    // Wait, what? Someone adding a line of code before your code changes how your nil check works?

This, I think, is the reason everyone uses "error" in their return values in go, even though using a specific type (like "*os.PathError") would make it much easier to know what possible error conditions a function could have.

Thanks!

Another way to explain the difference is the following:

    // case 1 is the same as
    var i int
    // inferred type of *MyErr because that's what returnsNilError returns
    var err *MyErr
    i, err = 0, (*MyErr)(nil)
    // err is of type (nil)(*MyErr)

    // case 2 is the same as
    // inferred type of error because os.Open returns that
    var err error
    err = os.Open("/tmp/file")
    var i int
    i err = 0, (error)((*MyErr)(nil))
    // err is of type (error)((*MyErr)(nil))

It's well known that an interface with a type is != nil (i.e. "var nilErr *MyErr = nil; var errIface error = nilErr" results in an 'errIface' that is not equal to an untyped nil).

I hope that explanation made sense to you.

It is not possible to always do this; `if val, err := ...; err != nil { } else { use(val) }` is a Bit Much.

The second := in case 2 should have raised a visual alarm. The fact that := even assigned for err a second time means inference could have happened, like

  err = (*MyErr)(nil)

  fmt.Printf("err is nil: %v\n", (*MyErr)(err) == nil)

  fmt.Printf("err is nil: %v\n", err == (*MyErr)(nil))

[1] https://github.com/golang/go/issues/30865

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 )

     type Thing string

     func (t *Thing) DoSomething() {
         if t == nil {
             fmt.Println("nil thing")
             return
         }
         fmt.Println(string(*t))
     }

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.

So here's where I do that:

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()
    }

    if foo == nil || reflect.ValueOf(foo)IsNil() {
        // handle the error
    }

    // Pass foo elsewhere

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

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.

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.

I disagree with the characterization that "Go expects you to be an expert in each one". That's what C does, with all its arcane implicit type conversion rules, memory models and UB traps lying everywhere. Coming from C and learning Go, it was refreshing, for instance, for integer overflow to be defined, and for it to be impossible to even compare `int32` to `int64` without a cast.

Which is why my actual proposal is along similar lines: The "magic" of comparing an interface to a bare `nil` contains a trap, so disallow it. If you ended up with something like this:

    if display == interface{}(nil) {
        ...
    }

Furthermore, I do stand by the comment I made in the discussion of my proposal regarding comparing `error` to `nil`: It turns out, probably the single most common pattern in Go is in fact not safe. Anyone anywhere could accidentally return `*MyError(nil)`, and suddenly all the error checking code all the way up the stack is completely broken. Yes, as you say, doing so would clearly be a bug; but the whole point of having a type-checked language is to be able to prevent this type of bug from cascading throughout the system.

Golang is still, in fact, my go-to language for new projects. And in most things it's just great -- I know how to use the type system to prevent all sorts of dumb mistakes, and 75% of the time, if it compiles, it Just Works. It's because Go is so good at preventing stupid errors that I find the "interface trap" so frustrating.

Until you correct your understanding, you will continue to find Go frustrating. nil and invalid are not the same thing. This is not a matter of opinion; it is how the language works. As long as you persist in thinking otherwise, you will remain confused.

If you checked whether err is nil by checking the interface value, and then penetrating the interface to see if the underlying concrete type is a pointer and than pointer is nil, that is a bug in the code doing that. You should never do that. It is never correct. It is objectively wrong to think of them as the same thing and to treat them the same, because they are not.

The correct solution is what I outlined; do not create the lying interface values in the first place. Then they can not propagate through your code and mess you up.

I'm not trying to debate whether Go is "right" or "wrong" here. It is perfectly acceptable to come to a correct understanding of how Go works and still think it is not the best way to build the language. Personally I'd still be happy to get non-nillable pointers, in the same way C# managed to add them later, so you can put me in that category myself. But you still have an incorrect understanding of how Go works, and I'm writing this not to defend Go but to try to save you that frustration. However, if you keep resisting the correct understanding, you will continue to be confused and frustrated and write bugs.

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.

Out of curiosity, what are the first two?

For me it's:

1. context.Context and putting everything under the sun there, untyped (which is a design fault, not just "people using it wrong")

2. reusing the looping variable in for loop

2.5. general awkwardness of go templating (I plan to write whole article about this, but too lazy to actually write it)

The language makes a big deal about memory management and data structure ownership, and as a result it’s sorta a pain to send stuff around on the stack, both at initialization and runtime. As a casual golang user, the context mess feels like an escape hatch to help with those problems. I’d prefer some GC compromises built into the language itself instead.

FWIW, I think request-scoped globals are terrible and is among the many things I hated about Flask. I think generics will largely solve this problem by allowing us to write generic middleware with a type-safe context and probably even functions to compose the middleware and automatically propagate the context variables.

My concern with its use in golang is that so much seems to get jammed into it. My experience with it is that it ends up being a holding pen for all the things that other systems do with initialization-time dependency injection / resource lookup sorts of things, but replaced with "oh just pass `ctx` as an argument everywhere".

From what I've seen of Go, it looks like that happens because of the state management idiomatic constraints of the language, which in turn seem GC-related.

In a garbage-collected language, I'd put a bunch of context-y things into some sort of request executor datastructure, the web framework would manage the initialization and retention of that, and request handlers would operate against those intialized data structures. That doesn't line up all that well with idiomatic Go, and so we end up needing to hide those sorts of things somewhere else. Enter context-abuse.

Of course, there are plenty of use cases where those sorts of patterns aren't relevant. Right tool for the job and whatnot.

Again, I'm a casual golang user at best -- I bet I'm missing some newer / better approaches, and would love to learn more about what they are.

What (I think) you are suggesting implies globally scoped, mutable variables that affect control flow dramatically. Sounds really hard to reason about and very munch anti-Go-explicitness. Context is the handle by which an outer context holds on to the nested ones.

The only non-bonkers way to have a handle like that would be to give every function a getContext function to query some kind of context tree and signal parent/children. This is totally against Go explicitness as well.

Maybe something like Scala's implicits to sugar it, but that's another not-go-looking pattern (although it would be nice)

It makes me want to port it to Rust, since so many of the major Rust templating libraries are Jinja copies or Handlebars copies.

Go authors themselves admit it might have been a design mistake, but is unfixable without breaking compatibility.

https://golang.org/doc/faq#closures_and_goroutines

> This behavior of the language, not defining a new variable for each iteration, may have been a mistake in retrospect. It may be addressed in a later version but, for compatibility, cannot change in Go version 1.

How does that work? The call-frame needs to be of fixed-size, but it's rarely knowable at compile time how many iterations a loop will perform. Unless we begin heap-allocating loop variables or treat each loop iteration as a function call (both of which will kill performance, I think), I don't see how this could work?

    for _, v := range values {
        v := v // create a new 'v'.
        go func() {
            fmt.Println(v)
            done <- true
        }()
    }

Probably, hence my "How does that work?" question.

I don't doubt your example--I know it works, but I'm struggling to understand how it manages to work, since (as I understand it) the closure presumably captures the environment, which presumably means taking the address to the new `v` which presumably is the same address for each loop iteration?

Whether a loop is involved is irrelevant, it comes up any time a value referenced a closure might outlast the environment which created the closure. That can happen in loops but also if conditions or normal function calls. The value might be promoted to the heap, but in trivial cases like the above it could be copied directly on the new goroutine's stack. Or if escape analysis says the closure can't outlast the environment, it will use the same stack.

   for _, v := range values {
        go func(v string) {
            fmt.Println(v)
            done <- true
        }(v)
    }

Do you think it's smart enough to do the trivial transformation of the first into the second?

Right, I understand the difference between a variable and a region of memory.

> If `values` is an array of strings, do you think the following makes a new string on the heap (that is, a new 16 byte heap allocation, internally pointing to the same variable-sized region of memory)?

No.

> Do you think it's smart enough to do the trivial transformation of the first into the second?

I guess I'm surprised that the semantics for the `v := v` in a loop thing are "this will get allocated somewhere else such that it's guaranteed not to be stomped on by another loop iteration". For example:

    for _, v := range []int{2, 4, 6} {
        v := v
        mut := sync.Mutex{}
        go func() {
            mut.Lock()
            fmt.Println(v)
            mut.Unlock()
            done <- true
        }()
        go func() {
            mut.Lock()
            v = 1
            mut.Unlock()
            done <- true
        }()
    }

1. Error handling. Not only is `if err == nil` (or my preferred `if _, err := foo(); err == nil`) a lot of noise, errors are completely opaque and might as well be strings.

2. Dependency management. It was a mess for a long time, go modules fixed a lot of issues in it's own way but still has problems (e.g. the "v2" workaround). My theory is they expected everyone, in some way, to "self-host" their modules (like how Java would have com.google.foo.bar, eventually everyone else would have "http://go.company/package", but GitHub became central repository of software for everyone and linking directly to packages was no longer sound).

Errors in modern Go are definitely not just strings.

Before wrapped errors in Go, this would have been an easier case to make against the language.

   foo := &FooError{}
   if errors.As(err, &foo) {
       fmt.Println("foo error!")
   }

Maybe it’s a good thing it’s not widely known, generics are trivial to discover.

    func processItem(item *string) {
      if item == nil {
        fmt.Println("nil item")
        return
      }
      fmt.Println(*item)
    }
    
    stuffToProcess := []string{"one", "two"}
    wg := sync.WaitGroup{}
    for _, item := range stuffToProcess {
      wg.Add(1)
      go func(s *string) {
        time.Sleep(1 * time.Millisecond)
        processItem(s)
        wg.Done()
      }(&item)
    }
    wg.Wait()
    // What do I print?

    [reuse.go:26] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM)
    25:    wg.Done()
  > 26:   }(&item)
    27:  }

https://play.golang.org/p/srAyKZN2NTH

I assume GGP's contrived example is to show the underlying mechanics of the issue.

It's inability to deal with cyclic references, the lack of overloading, the lack of package-level visibility and the lack of static functions all contribute to a lot of ceremony.

You end up with functions like newUserWithPassword and NewRole (casing intentional), instead of User::new() and Role::new()

You could just have a NewUser that takes a NewUserOpts which exposes a fluent interface. But again, a lot of ceremony.

You can use a package per type, but still no overloading, and you'll need a 3rd package to bridge the two if the reference each other.

After 4 years of Go, this is usually a design thing. Once I figured out how to separate and isolate domains of interest better, I stopped running into these issues entirely. My code, in and outside of Go, is much better for it. I also learned that much of "thread-safe" programming is taught this way, which makes sense.

I've been getting into Go for the past couple of months, but I still struggle with this.

https://play.golang.org/p/FUS2-CcHflX

The more I played with the code, the more I have to agree with the commenter. This is not obvious behavior.

You can also get into obscure behavior if your implementation does not have a pointer receiver. Take this code for example: https://play.golang.org/p/MjjGMAZL3-v How can I get a nil pointer immediately after I checked whether it was nil? This one, I do know has bitten me before. Infrequently, but it has.

    func (c *GZipComprosser) compress() []byte {
      if (c == nil) {
        return []byte{}
      }
      return []byte{c.internal}
    }

I don't think that's common at all (1) nor is it a sane burden to put on developers.

(1) https://github.com/golang/go/blob/master/src/bytes/buffer.go...

Better way is what you described, accept nil state as invalid, require user to handle it (user must initialize before using your method)

Or not.

1. Checking for nil receivers is rare and hardly idiomatic. (Although I have seen methods that are designed to work with nil receivers)

2. (*T)nil != nil is surprising.

A newbie looking at this code would not expect it to crash which is why I consider it a wart. In this specific case, the code was structured such that if the compressor was nil then no compression would happen. Then in some, hard to remember case, when you opted into gzip the whole thing would crash. I think I had been writing Go for about 18 months at that point and that snippet got past review. Ultimately the design had problems, (depending on `nil` to mean anything was probably dangerous and it's clearer to give NoCompression it's own type) so I'm not really arguing that the above snippet should work.

Once you understand why it's there it makes sense, but ultimately I think it's an unfortunate side effect of designing a language with nullable types.

I think newcomers are exposed to this behaviour now. For instance, it is step 12 in the tour of Go:

https://tour.golang.org/methods/12

I learnt Go before the tour was available and learnt a lot from going through the tour later on.

In Java land at some point we were taught to never use null anywhere, instead use the null object pattern.

And in Go I'd like to extend that, never call a method on a nil receiver if you can avoid it. I think it should be up to the caller to do the nil check, IF the receiver can ever be nil.

Hahaha oh you sweet summer child. It's considered a feature.

And since typed nils will never get fixed despite being an actual wart (though the FAQ tells you you're holding it wrong you idiot: https://golang.org/doc/faq#nil_error) there's no chance nil receivers will get removed from the language.

you also end up with things like this: https://play.golang.org/p/wgaLGOqFDZ9

  func main() {
      var c Compressor  // signal do nothing
      DoCompress(c)
      c = &GZipComprosser{}  // normal call
      DoCompress(c)
  }