variable set in a loop but unused not failing compilation

98 views
Skip to first unread message

Tiago de Bem Natel de Moura

unread,
May 6, 2024, 10:20:26 AM5/6/24
to golang-nuts
Hello,

We had the bug below in production:
https://go.dev/play/p/-jewy7e7UcZ

Look at the `opt` variable inside `listGithubPullReviews`, it's set multiple times (inside the loop) but never used... it was supposed to be passed as the last argument of `ListReviews()`.

Why Go compiler is not giving an error for this case? AFAICS all of those `opt.Page = resp.NextPage` inside the loop cannot make any side effects, then it looks safe to assume the variable is only set and never used?

So, simplifying the problem, this is the non-failing case: https://go.dev/play/p/FLAIlVx_sSG

But if you change from a struct to a plain int, then the compiler gives: `./prog.go:6:2: a declared and not used`

Luckily, we were using a `ctx` with timeout otherwise it would be an infinite loop. The thing was tested but only for cases where all results came in a single page, then the loop aborted in the first iteration. I think this could be detected by the compiler, no?

Axel Wagner

unread,
May 6, 2024, 10:35:02 AM5/6/24
to Tiago de Bem Natel de Moura, golang-nuts
Hi,

I'll note that this has nothing, really, to do with the loop, but with the fact that you are assigning a struct field.
For a simpler example, compare this: https://go.dev/play/p/MmR-AhOUQH3 with this: https://go.dev/play/p/Y1uoI8thYuV
If you only write to the entire variable, the compiler complains about that variable not being used. But if you set a field, the failure goes away.

This compilation error is based on this sentence from the spec:
> Implementation restriction: A compiler may make it illegal to declare a variable inside a function body if the variable is never used.
Note that it says "may" make it illegal. It's also not really defined what it means for a variable to be "never used".

TBH I don't really like these implementation-defined compilation failures. But it would be difficult to make them more strict. Perhaps this case could be a vet warning.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/9a2b5179-f083-4365-b0c6-e876f3fe6950n%40googlegroups.com.

Tiago de Bem Natel de Moura

unread,
May 6, 2024, 7:13:38 PM5/6/24
to golang-nuts
Hi Axel,

Thanks for the explanation, it makes sense. Yes, I think it would be a good case for a `vet` warning.
Reply all
Reply to author
Forward
0 new messages