Unreachable code: error or warning? [closed]
Asked Answered
A

12

25

This is a language design question:

Do you think unreachable code (in programming languages in general) should raise a warning (i.e. "report problem and compile anyway") or an error ("refuse to compile")?

Personally I strongly feel it should be an error: if the programmer writes a piece of code, it should always be with the intention of actually running it in some scenario. But the C# compiler for example seems to disagree with this and merely reports a warning.

Note: I realize good dead code detection is a very difficult problem, but that is not the focus of this question.

Here are some examples of pieces of code where some statements are clearly unreachable:

return;
foo();

--

throw new Exception();
foo();

--

if (...) {
  return;
} else {
  throw new Exception();
}
foo();
Aberrant answered 17/2, 2010 at 13:4 Comment(0)
T
17

Generally speaking it should be an error.

One exception comes to my mind however:

if (false) {
  doStuffThatITemporarilyDisabled();
}

Some developers might complain if your compiler denies compilation for code like that.

Trivium answered 17/2, 2010 at 13:6 Comment(6)
Isn't this what comments are for?Aberrant
@Aberrant Using comments to comment out code is bad practice because the compiler cannot warn you if you forget to remove them.Germanize
Also: comments prevent the IDE from applying refactorings inside the disabled code.Trivium
The refactoring is a very good argument I hadn't thought about before! Thanks. :-)Aberrant
I disagree. An error means "I can't compile". A warning is "I can compile, but I think you did something wrong". Under those definitions, this is most definitely a warning.Hyperplasia
Furthermore, because of the Halting Problem and co., you can determine for sure that code will be run...so at best you're looking at heuristics. Improve the heuristics, and bam!...suddenly old code no longer compiles, and your compiler is not backwards compatible.Hyperplasia
W
37

An error means that the compiler is physically unable to deal with your code.

A warning means that the compiler is capable of dealing with your code, but it believe that what you have written is wrong in some way.

It seems pretty clear cut to me - it should be a warning.

Besides, what about the case where I've decided to shorten a method for debugging purposes:

public bool ShowMenu()
{
    return true;
    /* The standard implementation goes here */
}

I know its wrong, but for the compiler to ask me to also comment out that code would just be a pain.

West answered 17/2, 2010 at 14:11 Comment(0)
G
28

It is very common to create dead code intentionally when debugging - a compiler that prevents this is unlikely to be popular with its users.

Germanize answered 17/2, 2010 at 13:6 Comment(2)
This is true, but it's not an answer.Cribbing
Unfortunately, Java and C# are kind of popular :)Ellington
T
17

Generally speaking it should be an error.

One exception comes to my mind however:

if (false) {
  doStuffThatITemporarilyDisabled();
}

Some developers might complain if your compiler denies compilation for code like that.

Trivium answered 17/2, 2010 at 13:6 Comment(6)
Isn't this what comments are for?Aberrant
@Aberrant Using comments to comment out code is bad practice because the compiler cannot warn you if you forget to remove them.Germanize
Also: comments prevent the IDE from applying refactorings inside the disabled code.Trivium
The refactoring is a very good argument I hadn't thought about before! Thanks. :-)Aberrant
I disagree. An error means "I can't compile". A warning is "I can compile, but I think you did something wrong". Under those definitions, this is most definitely a warning.Hyperplasia
Furthermore, because of the Halting Problem and co., you can determine for sure that code will be run...so at best you're looking at heuristics. Improve the heuristics, and bam!...suddenly old code no longer compiles, and your compiler is not backwards compatible.Hyperplasia
A
8

I think a warning is appropriate. Then the user can decide to use your OTHER switch that says "treat all warnings as errors" for when he does a Release build. Empowering the developer is always better than forcing your essentially random choices onto him.

Azeria answered 17/2, 2010 at 13:15 Comment(0)
T
8

I think it should be a warning.

Generally speaking, the common rule is that 'you should treat warnings as errors'. If I unintentionally write unreachable code, I will see it in the warnings, and fix it anyway.

However, sometimes, like Neil says, you intentionally create dead code, for debugging purposes or whatever reason. I'd really, really hate it if the compiler would not let me do that.

Tactual answered 17/2, 2010 at 13:23 Comment(0)
F
4

If you language specifiction:

  • Considers dead code to be an error
  • Doesn't support C-style "text-level" preprocessing
  • Lacks block-style comments that can nest with line-style comments

... then it will truly suck. It's imperative to be able to disable large blocks of code without having to cut them out of the file physically.

Flagrant answered 17/2, 2010 at 13:10 Comment(2)
Per your statement (but not my opinion), Java sucks. See the Java Language Specification for J7SE (emphasis original): "It is a compile-time error if a statement cannot be executed because it is unreachable." C# is different: "A warning is reported if the compiler determines that a statement is unreachable. It is specifically not an error for a statement to be unreachable."Mccombs
@DragonLord Why is it a problem if his premise leads to the logical conclusion that Java sucks? His premise is opinion, but if true, then Java does actually suck.Cleanthes
H
4

I'd much prefer (provably) unreachable code to produce either a warning or a notice (like a warning, only without negative connotations). Mainly because this makes automatic code generation slightly easier.

Husain answered 17/2, 2010 at 13:51 Comment(0)
N
2

A good reason for having that code is during the development phase: you want to temporarily disable certain code, but you still want this code to be checked on syntax.

for example:

int i = 2, j = 3;
int result = 0;

// FIXME: Commented out for now because I have to recheck calculation
if (false) {
  result = i*2+j+3+i*j;
}

System.out.println("Result of difficult calculation = "+result);

If you put the list "result = i*2+j+3+ij;" just in / comments */, you're pretty sure that when you remove or rename certain variables (e.g. i), you do not get an error.

Nutrilite answered 17/2, 2010 at 13:35 Comment(0)
C
2

It should be a warning by default with a compiler flag that allows it to be treated as an error. Requiring it to be unchangeably one or the other is inconsiderate.

Convulsant answered 17/2, 2010 at 13:43 Comment(0)
B
2

I think it should be a warning because of the same reason as Roalt said. A good compiler should also exclude this code from being compiled.

Bashuk answered 17/2, 2010 at 13:46 Comment(0)
D
2

I an still not clear why Java is implementing the dead code as an error. I use C and Objective-C, which have preprocessors, thus I can easily use preprocessor directives to enable (or mask) an function/method, like:

// C code
#define ENABLE_FOO //Comment off this to turn off foo(void);
int foo(void)
{
#ifdef ENABLE_FOO
    // Do actual stuff.
    int returnVaue = 2;
    // ...
    return returnValue;
#else
    return 0; // Turned off
#endif
}
// Compiling using clang, enforcing dead code detection:
// clang main.c -Wall -Werror

or, like in Objective-C, with reflection available:

// Class
#define MYCLASS_ENABLE_FOO // Comment off to enable -[MyClass foo]
@interface MyClass : NSObject
#ifdef MYCLASS_ENABLE_FOO
- (int)foo;
#endif
@end
// The like is done in implementation.
// Invoking:
int main(int argc, char **argv)
{
    MyClass *object = [[MyClass alloc] init];
    int value = ([object respondsToSelector:@selector(foo)]) ? // Introspection.
                [object foo] : 0;
    printf("Value: %d", value);
    return 0;
}
Digitalin answered 10/2, 2013 at 10:8 Comment(0)
C
2

The answer is that the final decision as to whether unreachable code triggers an error or a warning should be left in the hands of the developer. The compiler should allow either.

In the case of production release code the balance favours it being an error. No production program should contain unreachable code.

In the case of debugging/testing code the balance favours it being a warning. It is extremely convenient to be able to disable parts of the code for debugging or testing.

The above refer only to hand-authored code. In the case of code that has been generated by an automated tool, the balance favours it being a warning or ignored. The generation of code that includes unreachable portions has no particular adverse consequences and may be extremely hard to avoid.

Cribbing answered 15/3, 2014 at 13:20 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.