Compiler doesn't detect unreachable code?

[Nate Finch]

I was surprised to note that the compiler doesn't return an error when it sees unreachable code even in incredibly simplified cases, thusly: http://play.golang.org/p/YpM8K7sJ3B

if false {

fmt.Println("You'll never see this, playground")

} else {

fmt.Println("Hello, playground")

}

I presume the compiler is smart enough to drop the code for the if branch... but shouldn't it return an error? This is obviously programmer error.

[steve wang]

It's not necessarily an error. It might be a debug trick in some cases. 

[minux]

The compiler will remove dead code in simple cases like this.

using if false {} to skip a block of code is a standard way to comment out a huge block of code,

it's especially useful as Go doesn't allow nested comments and C's "#if 0" tricks.

Even gcc in -Wall -Wextra mode won't give a warning for code like this:

#include <stdio.h>

int main() {

if(0)

puts("You'll never see this, playground");

else

puts("hello, playground!");

return 0;

}

btw, it's even a recommended C practice to use if (FEATURE_ENABLED) to conditional include/exclude

specific code blocks because it makes sure the compiler will check both branch of code for any errors

while #ifdef FEATURE_ENABLED won't make the compiler check the code in the disabled branch.

[Nate Finch]

Hrmph. This seems like a place where the compiler could help prevent programmer error. I didn't think "I use this as a debugging technique" was a valid answer in response to incorrect code (see: unused variables).  The code is clearly incorrect as a part of a final program, which is why the compiler drops it.

The C# compiler warns about this, which has saved me on several occasions when I inadvertently edited valid code to cause a branch never to be called, and that's why I was surprised not to see an error from Go's compiler, since Go's compiler has always seemed to be proactive about preventing obvious programmer error.

And to be honest, I don't really care what C does. We're not programming in C.

[Maxim Khitrov]

There is nothing incorrect about this code and it's not an obvious
mistake. It's a perfectly valid way of excluding code paths, be that
for debugging or any other reason. The compiler doesn't know your
intentions and should not assume that you've made a mistake when you
haven't. Go compilers don't produce warnings, so that's not an option
either. For reasons why, see the following FAQ entry:

http://golang.org/doc/faq#unused_variables_and_imports

- Max

> --
> 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.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[Patrick Mylund Nielsen]

Is this an obvious programmer error?

const debug = false

func main() {

        if debug {

                log.Println("Launching application")

        }

        ...

}

--

[Nate Finch]

On Tuesday, February 19, 2013 9:24:47 AM UTC-5, Patrick Mylund Nielsen wrote:

Is this an obvious programmer error?

const debug = false

func main() {

        if debug {

                log.Println("Launching application")

        }

        ...

}

yes, just as much as this is:

func main() {

    var debug bool

    ...

    fmt.Println("Hello, playground!")

} 

// prog.go:6: debug declared and not used

[minux]

On Tue, Feb 19, 2013 at 10:12 PM, Nate Finch <nate....@gmail.com> wrote:

Hrmph. This seems like a place where the compiler could help prevent programmer error. I didn't think "I use this as a debugging technique" was a valid answer in response to incorrect code (see: unused variables).  The code is clearly incorrect as a part of a final program, which is why the compiler drops it.

The C# compiler warns about this, which has saved me on several occasions when I inadvertently edited valid code to cause a branch never to be called, and that's why I was surprised not to see an error from Go's compiler, since Go's compiler has always seemed to be proactive about preventing obvious programmer error.

And to be honest, I don't really care what C does. We're not programming in C.

Go absorbs much from C. And as Go intentionally drops C's preprocessor support,

the only way (if not using build tags), is to use if conditional to selectively disable/enable

a code block and using a build tag for a simple statement is simply overkill.

Even the standard library uses this technique, for example:

http://tip.golang.org/src/cmd/fix/typecheck.go?h=if+false#L316

http://tip.golang.org/src/cmd/yacc/yacc.go?h=if+false#L1538

Not to mention there are other occurrences in test code.

Go's runtime code also make use of this technique extensively to disable debugging

code in normal builds.

http://tip.golang.org/src/pkg/runtime/mgc0.c#L18

http://tip.golang.org/src/pkg/runtime/mgc0.c#L610

[minux]

On Tue, Feb 19, 2013 at 10:12 PM, Nate Finch <nate....@gmail.com> wrote:

The C# compiler warns about this, which has saved me on several occasions when I inadvertently edited valid code to cause a branch never to be called, and that's why I was surprised not to see an error from Go's compiler, since Go's compiler has always seemed to be proactive about preventing obvious programmer error.

I don't like the compiler giving warning for obvious and trivial dead code like this.

but gcc (even without -Wall -Wextra) does give warning for non-obvious dead code for this:

int f(unsigned char x) {

if (x < 0)

return 42;

return x > 255 ? 0 : 1;

}

This is helpful warning (because the programmer might don't expect a conditional like this

to be a const due to limited value range), but warning for if (0) is not.

[Nate Finch]

BTW, that uint < 0 code compiles just fine in Go.

It seems inconsistent to me to have an error about unused variables, but not have one for entire sections of code that are unused. 

I don't want to beat a dead horse. If no one else agrees, so be it. 

[Patrick Mylund Nielsen]

I don't disagree that they are similar, just that the former isn't useful, and the latter is. Sure, you can argue that you "just want to leave some dev vars in for testing", but the problem is you leave them in. That's not really the same thing as having the ability to compile with a debug flag when you're troubleshooting an issue, or compiling without, and getting the same performance you would have gotten if all of the code inside and including "if debug { ... }" clauses was removed from your source tree.

--

[Rémy Oudompheng]

No: your example has inconditionnally a problem. The "const debug =
false" example is different because the value of debug might depend on
build tags (hence giving a conditional error, which is weird given
that both cases make sense).

Rémy.

[Dave Cheney]

I suggest you review the md5 package for a non contrived, non trivial use of this feature.

http://tip.golang.org/src/pkg/crypto/md5/md5block.go

[minux]

On Wed, Feb 20, 2013 at 4:58 AM, Dave Cheney <da...@cheney.net> wrote:

I suggest you review the md5 package for a non contrived, non trivial use of this feature.

http://tip.golang.org/src/pkg/crypto/md5/md5block.go

Good example! 

[Nate Finch]

Very good example, Dave.  I concede the point.

Final note - somewhat different perhaps. What about statements after a return?

http://play.golang.org/p/ur4xG0Zd8n

[Dan Kortschak]

On Tue, 2013-02-19 at 17:33 -0800, Nate Finch wrote:
> Final note - somewhat different perhaps. What about statements after
> a
> return?
>

I use returns, continues and breaks that make code unreachable to
isolate parts I am working on just the same way.

[bradp...@gmail.com]

Just wondering if there was some sort of consensus on this (dead code in cases where it really isn't going to come alive - like after a return).  IMO it seems contrary to the approach employed toward unused variables - where it's an error.  One could argue that those are useful too while your editing your code...

If it's just an open issue and it is what it is, then fine. But as new comer to the language it struck me as odd and not totally in keeping with the rest of the Go philosophy (which is, by and large, awesome).

[Rémy Oudompheng]

Le 1 oct. 2013 06:10, <bradp...@gmail.com> a écrit :
>
> Just wondering if there was some sort of consensus on this (dead code in cases where it really isn't going to come alive - like after a return).  IMO it seems contrary to the approach employed toward unused variables - where it's an error.  One could argue that those are useful too while your editing your code...
>

There is a consensus: it is not an error and it is not going to change. You are also replying to a very old conversation which may not have anything to do with the current situation.

> If it's just an open issue and it is what it is, then fine. But as new comer to the language it struck me as odd and not totally in keeping with the rest of the Go philosophy (which is, by and large, awesome).
>

That's unfortunate. The rules are not going to change in Go 1.x because libraries that compile under Go 1.0 must continue to do so.

There is now a warning in govet about this.

Rémy.

[RickyS]

Nearly on-topic:

1. Is there a tool (lint? ) for finding dead code?
2. Is there a tool for finding un-called functions?  This is harder because
   packages need to have uncalled exported functions.
   
3. Is there a tool to find uncalled functions in compiled main programs (even in imported packages)?
4. Is there a tool to find uncalled un-exported functions in packages?

Profiling can get this information, for some profilers.  Will pprof do this?

[Dan Kortschak]

Rémy has this: github.com/remyoudompheng/go-misc/deadcode

[RickyS]

1.
Simple constant Boolean expressions should be optimized away.  The blank identifier doesn't help, as currently defined.

If the Go Authors are truly addicted to the drug of no warnings, they should make complex constant Boolean expressions a hard error, still permitting single-word cases:

debug := false
if debug {...}

2.
By analogy with 'for {}' being endless, 'if {}' could be allowed.  But that's only half the problem.

3.
My bug was this:

   byte x
   for x = 0; x <=255; x++ {
      Do stuff with ASCII character arrays…
   }

I was really puzzled that it was an infinite loop — for a while. (pun not intended).

4.
Perhaps since the null expression is shorthand for true, it's negation, in the same context, should be permitted as the shorthand for false?

if ! {
   fmt.Println("You'll never see this!")
}

      ☺

I'm beginning to like this idea...

[RickyS]

Github denies any knowledge of that link...

[Rémy Oudompheng]

it is made for unused identifiers, go vet -unused now does it better.

Rémy.

2013/10/1, Dan Kortschak <dan.ko...@adelaide.edu.au>:
> Rémy has this:
> github.com/remyoudompheng/go-misc/deadcode<http://github.com/remyoudompheng/go-misc/deadcode>

>
> On 01/10/2013, at 8:12 PM, "RickyS"

> <rickys...@gmail.com<mailto:rickys...@gmail.com>> wrote:
>
> Nearly on-topic:
>
> 1. Is there a tool (lint? ) for finding dead code?
> 2. Is there a tool for finding un-called functions? This is harder

> because
> packages need to have uncalled exported functions.

> 3. Is there a tool to find uncalled functions in compiled main programs
> (even in imported packages)?
> 4. Is there a tool to find uncalled un-exported functions in packages?

>
> Profiling can get this information, for some profilers. Will pprof do
> this?
>

[tomwilde]

https://github.com/remyoudompheng/go-misc/blob/master/deadcode/deadcode.go

[Dan Kortschak]

Thanks for the heads up.

Dan

[Rob Pike]

go vet -unused doesn't exist. I deleted the experiment because the approach was not succeeding.

-rob

[RickyS]

Syntax is: go tool vet -unreachable