
Golang landmines - kornish
https://gist.github.com/lavalamp/4bd23295a9f32706a48f
======
oppositelock
I've been doing quite a bit of Go programming for a while now, and I've
internalized all these behaviors, but this one gets me every time:

    
    
       var err error
       if true {
           foo, err := someFunction()
           // err gets shadowed
       }
    

It's particularly annoying because if both foo and err are defined in the
outer scope, you get a compiler error that no new variables are on the left,
so you tend to forget that removing one shadows the other.

~~~
kyrra
Not sure it would help you but adding "go vet -shadow" to some kind of test or
build script may help you detect these problems.

[https://golang.org/cmd/vet/](https://golang.org/cmd/vet/)

~~~
leetrout
This.

If anyone is using VIM or Atom please please please install the Go helpers,
too. Atom's go-plus is awesome.

The team I work with- I beg them to use linters and go-plus and avoid these
pitfalls like shadowing err. Instead most of them will run the linter, not fix
the errors, then defend poor practices like shadowing and returning private
types from exported methods.

------
Animats
Languages probably shouldn't allow shadowing, certainly not within a single
module. Think of the maintenance programmers.

"Defer" was probably a bad idea. It's a run-time construct - you're adding to
a to-do list run at the end of the function. It's not scope oriented.
Something like Python's "with" clause would have been both simpler and more
useful. "with" calls "_enter" on an object when you enter a scope, and "_exit"
when you leave, no matter how you leave. All the file-like and lock-like
objects support "with". Python's "with" even supports exceptions properly - if
you're handling an exception when "_exit" is called, the "_exit" function gets
told about the exception, so it can clean up and then re-raise the exception.
If an "_exit" itself needs to raise an exception, that works, too.

"Defer" is blind. With "defer", nobody ever finds out that the close failed to
write the end of the file.

~~~
donatj
> "Defer" is blind. With "defer", nobody ever finds out that the close failed
> to write the end of the file.

This is simply false and a misconception held by people who haven't spent a
lot of time with the language. You not only can, but absolutely SHOULD handle
errors in defers.

Using named return parameters on your method, your error return is still in
scope and can be set. This is how you properly handle errors on defers.

See:
[https://play.golang.org/p/kPx3ipkMhF](https://play.golang.org/p/kPx3ipkMhF)

~~~
Animats
That is so cool. And confusing. And likely to lead to errors.

Error handling should not require l33t features. (This is my big argument with
both Go and Rust. The stuff they use to get around not having exceptions is
worse than exceptions.)

~~~
donatj
I wouldn't really call "return values" a "l33t feature".

Exceptions are absolutely a "l33t feature". They break whatever is currently
happening. They are an uncontrolled return working its way up your structure.

While you're attempting to recover from that Exception I gracefully catch my
error and can even use the multiple return values that came back successfully
from my function, despite there being an error.

------
retroencabulato
> Everyone expects these values to be scoped inside the loop, but go reuses
> the same memory location for every iteration.

I don't see this as surprising as a C++ user. Who is 'everyone' here?

~~~
parenthephobia
In Ruby, you could do:

    
    
        10.times.map do |i|
          -> { puts i }
        end.shuffle.each(&:call)
    

This makes 10 lambdas which print the current value of the loop and then calls
them in random order. Each puts sees a different i.

I don't think the behaviour in the article is really a problem with loop
variables, but with _defer_. It is odd that defer packages up the function and
its arguments when it is defined. Deferring printing i remembers the value of
i at that time, whilst deferring printing &i remembers the (unchanging)
address of i.

OTOH, having defer remember the values makes things like

    
    
        f = open("file1")
        defer f.close()
        ... do stuff ...
        f = open("file2")
        defer f.close()
    

close both files, which is probably what the programmer expected. I think
that's pretty horrible code, but either way some people are going to find the
results surprising.

Go's "problem" is that it can't efficiently give i a new address in each
iteration: it'd have to put each one in a new memory location.

~~~
enneff
> Go's "problem" is that it can't efficiently give i a new address in each
> iteration: it'd have to put each one in a new memory location.

It's not an efficiency issue. How do you think Ruby does it?

~~~
perfmode
Then for loops would be linear in space.

~~~
parenthephobia
The block inside the loop must be kept in memory whilst there are still
anonymous functions which reference the per-iteration loop variables. Loops
_are_ linear in space if such functions are being generated and retained in
each iteration. If they don't, then iteration stack frames are cleaned up by
GC.

This applies to each-style loops. Actual Ruby for loops don't have this
behaviour.

    
    
        a = []
        for i in 1..100
          a << ->{ puts i } # append, to a, a function that prints 'i'
        end
        a[0].call()
    

This prints 100, since the scope of i covers the entire loop, and not each
iteration. They are, however, much faster than each-style loops, since they
don't have to make a stack frame for each iteration.

------
perfmode
It should be stated that entire teams build large systems with Go on a daily
basis and don't step on these.

The community of professional Go programmers has arrived at practices that
avoid such issues.

However, I wonder how many new programmers fall off early because of these
corner cases.

~~~
MaulingMonkey
> It should be stated that entire teams build large systems with Go on a daily
> basis and don't step on these

I strongly suspect they do step on them, and then simply learn the behavior at
that point, and then get on with their lives.

> The community of professional Go programmers has arrived at practices that
> avoid such issues.

I'd be interested in knowing those practices.

Shadowing I'd assume is "solved" with warnings (as errors)? I don't know
enough Go to know how you'd effectively avoid the non-nil interface to nil
instance issue, short of carefully checking for nil on every conversion. As
for the for loop scopes, do you just declare new variables in the local
scopes? Interestingly, I expected the "WRONG" behavior in 2 of the 3 for loop
examples... and C# actually changed their behavior (in C#5?) from "WRONG" to
"OK" with for-each loops.

~~~
jzelinskie
>Shadowing I'd assume is "solved" with warnings (as errors)?

Yep, shadowing variables are caught by the linters built into every golang
package for major editors. Using these linters is built into the culture and
as a result they tend to crop up less than you think for such a glaringly
obvious issue.

>I don't know enough Go to know how you'd effectively avoid the non-nil
interface to nil instance issue, short of carefully checking for nil on every
conversion.

You let it panic. This is a programming error, which is what panics are for.
If you caught the error, the proper thing to do would be call panic manually.

>As for the for loop scopes, do you just declare new variables in the local
scopes? Interestingly, I expected the "WRONG" behavior in 2 of the 3 for loop
examples... and C# actually changed their behavior (in C#5?) from "WRONG" to
"OK" with for-each loops.

This is subjective, and easily testable. Most early Go developers learn to
test their understanding of "for loop scoping" after they implement an
infinite loop that expects `defer f.Close()` to close the resource, but
instead it builds up until the kernel kills the process for not releasing file
descriptors.

\---

I was recently surprised to find out that `type MyError error` and switching
on types like `switch variable.(type)` can be a bit scary [0]. Basically you
should always define your new error type from a string and never attempt to
re-use the builtin error type. In retrospect it makes sense, but you'd naively
think this is the proper way to create a new error type.

[0]:
[https://play.golang.org/p/3BSKu8Cd4o](https://play.golang.org/p/3BSKu8Cd4o)

~~~
smarterclayton
I don't consider the linters enough to defend against shadowing - on
Kubernetes we find one or two err shadowing bugs a month
([https://github.com/kubernetes/kubernetes/search?p=1&q=shadow...](https://github.com/kubernetes/kubernetes/search?p=1&q=shadowing&type=Issues)
is just the ones for a trivial search) that can sometimes be quite serious (I
fixed one last week that leaked scheduler workers -
[https://github.com/kubernetes/kubernetes/pull/32643](https://github.com/kubernetes/kubernetes/pull/32643)).
Being extremely strict on shadowing has helped some, but it also leads to code
which looks non-idiomatic and is slightly worse to read and review.

Taking address of a loop variable has also come up several times, although
linters have many fewer false positives there.

------
kowdermeister
Maybe it's just me, but in these cases when I encounter some weird behavior in
a language I assume that I don't know enough of the language. It's actually
not the designers fault that they are there, but my knowledge that requires
more info.

------
dpc_pw
Golang is truely modern C. Popular, "simple" (in the twisted sense) and
riddled with landmines. :)

~~~
kjksf
Tell me the language that you're using and I'll find multitude of warts and
"landmines".

2 out of 3 listed in the article (scoping of loop variables and variable
shadowing) is the same in pretty much every other language, for good reasons.

The semantics of nil interface are surprising on first encounter, but when you
think about other possible semantics, it turns out it's the best solution. Or
at least I haven't seen a proposal for how it could behave better.

~~~
pcwalton
> Or at least I haven't seen a proposal for how it could behave better.

Java and C# just have null interface values compare equal to null. I've never
seen this cause problems and have never heard any complaints about this.

~~~
_ph_
But you can not call methods on null interface values. In Go that is perfectly
reasonable and safe to do.

~~~
pcwalton
That doesn't matter. Calling methods and performing comparisons are two
different things. I covered this here:
[https://news.ycombinator.com/item?id=12522941](https://news.ycombinator.com/item?id=12522941)

------
tptacek
By "never do what GetACat()" does, I think the author means "always return
structs, not interfaces".

~~~
TheDong
That doesn't work.

Here's the even more subtle version of that bug:
[https://play.golang.org/p/IVQ9MwyDGP](https://play.golang.org/p/IVQ9MwyDGP)

What you actually need to be careful to do is _always return interfaces when
possible_ because mixing interfaces and structs breaks shit.

You then also have to _always return explicitly 'nil' when you mean nil_.
That's what GetACat messes up; if you want to return nil from a function that
returns an interface, never return a struct you know to be nil, always return
the literal value nil.

~~~
bluejekyll
Can someone explain what a nil interface is and why it's useful?

~~~
parenthephobia
It's not clear which kind of nil interface you mean. :)

In Go, interface variables are a pair of values: a pointer to the concrete
value stored in the variable, and a pointer to the type of that concrete value
(i.e. the actual struct).

So there are essentially three kinds of nil in Go: a raw nil pointer, an
interface variable with both nil value and type pointers, and an interface
variable with a nil value pointer, but a non-nil type pointer.

One way that the two "interface nils" differ is that, to follow the article's
example, if you have a Cat variable containing a nil *Tabby, you can invoke
Meow on it, and it'll print meow. This works because Tabby's Meow doesn't try
to dereference its pointer-to-Tabby. However, if you have a Cat variable
that's completely empty, invoking Meow on it will fail.

~~~
thaumasiotes
What is the conceptual difference between a raw nil pointer, e.g. var t *Tabby
= nil, and an interface with nil value and non-nil type? The raw pointer also
has nil value and non-nil type.

~~~
Matthias247
The difference is that on a nil-interface you can't call a method, it will
panic. On a non-nil interface it will always call the method, wether a nil-
pointer is stored as the receiver or not. It a nil pointer is stored it will
call the method will it, and the method may still do something useful.

It can be theoretically be used to implement some interfaces without any
backing object at all (singletons, pure functions, ...), which is a difference
to most other programming languages.

~~~
thaumasiotes
You're explaining the difference between two things, but not the two things I
asked about.

~~~
Matthias247
Sorry - the question is not immediatly understandable. If you mean how 'var t
*Tabby = nil' is different from the interface representation: For this one no
type information is stored at runtime. It's only a single pointer. Whereas an
interface at runtime is always represented by 2 fields, a pointer and a type
field.

~~~
thaumasiotes
Thanks. It didn't occur to me that non-interface types might not actually
carry type information.

------
zzzcpan
For me channels and append() are the biggest language level landmines. Not so
big are nils, :=, lack of panic when you go beyond last element of the slice,
i.e. ([]int{0,1})[2:].

Libraries are a lot worse though, they are minefields.

~~~
sigill
Do you propose having another syntax for getting the zero-length slice just
past the last element? The [2:] syntax giving you {} is a pretty natural
progression from [0:] giving you {0, 1} and [1:] giving you {1} in your
example.

------
sheeshkebab
I've run into for loop go routine one just the other day... reminded me of
groovy and it's closure scopes too.

~~~
35bge57dtjku
Isn't that the same shitty problem Javascript has?

------
coldcode
As a Swift programmer Go programming seems riddled with a need to be perfect.
I'd rather the language help me not make errors rather than assume I am
perfectly aware of all the gotchas. Swift isn't perfect but it does make
things less likely to confound you later.

------
mmargerum
I don't know if I'd call this a land mine, but I wish it didn't work because
it confuses beginners.

You can type

for i:= range myvalues

In this case i is the index of the value. I'd rather they didn't allow this
and instead you would just type

for i,_ := range myvalues

------
blt
Seems that the explicit lambda capture in C++ is a good way to avoid the first
issue. Implicit capture of closure variables is kinda scary when you think
about it.

------
oconnor663
I would add taking a value receiver when you meant to take a pointer receiver.

------
karma_vaccum123
If only one could go back in time and fix the weird overloading of nil for
pointers and interfaces...it is the one breaking change to the Go1 language
guarantee that seems worth it.

~~~
kenshaw
Could you explain what you mean? I don't think many experienced Go programmers
would agree with you, so I'm curious as to your rationale.

~~~
dilap
I don't think it's worth breaking compat, but I agree it would've been nice
and saved a ton of confusion.

Right now a pointer to a type that is zero is called "nil". Similarly, an
interface that contains zero for both the type and the value is called "nil".

This is really confusing because

    
    
        var foo SomeType = nil
        var bar SomeInterface = foo
        fmt.Println(bar == nil) // prints false, which is confusing
    

If instead you called the zero-value of interface something else, say "unset",
it would be a lot less confusing.

    
    
        var foo SomeType = nil
        var bar SomeInterface = foo
        fmt.Println(bar == unset) // prints false, which makes sense, because bar is set -- to (nil, SomeType)
    

I first encountered this idea in this reddit thread:

[https://www.reddit.com/r/golang/comments/4injtt/understandin...](https://www.reddit.com/r/golang/comments/4injtt/understanding_gos_nil_value/d2zkmyu)

~~~
kjksf
It's hardly a better solution.

In Go nil is a special identifier that represents a "zero value" for several
types (pointers, interfaces, functions, slices, channels).

"unset" would be a "zero value" for interfaces so that nil could be "interface
that has a type but whose value is an zero value of a pointer or function or
slice or a channel.

This is inconsistent - now you have 2 identifiers to represent a concept of
"zero value".

~~~
Someone
But they are different.

go has _three_ different concepts: what is now called _nil_ has instances from
two classes, let's call them _nil.type_ and _nil.interface_.

Discussion is about whether it would be better to require programmers to
explicitly specify the _.type_ or _.interface_ part in the code they write, or
to let the compiler infer it from its arguments (with the special case that it
should assume that, in _nil == nil_ , both have the same meaning, so that it
evaluates to _true_ )

I think I would favor having two separate names in my source code, with the
caveat that the compiler should make it illegal to write _x == nil.interface_
if x is a type and vice versa, but I know too little about go to claim that's
really a good idea, and also think it would change if go got generics (and
maybe, the code generation thing it has now in place of generics ( _go
generate_ ) already would change my opinion, if I took time to think it over)

~~~
eternalban
I should scroll before I post!

We're in agreement but I think <type>.<value> is a somewhat universal
syntactic construct.

