views:

154

answers:

10

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();
+8  A: 

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

anon
+4  A: 

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.

Joachim Sauer
Isn't this what comments are for?
Martijn
@Martijn Using comments to comment out code is bad practice because the compiler cannot warn you if you forget to remove them.
anon
Also: comments prevent the IDE from applying refactorings inside the disabled code.
Joachim Sauer
The refactoring is a very good argument I hadn't thought about before! Thanks. :-)
Martijn
A: 

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.

unwind
+3  A: 

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.

No Refunds No Returns
+4  A: 

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.

Razzie
A: 

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+i*j;" just in /* comments */, you're pretty sure that when you remove or rename certain variables (e.g. i), you do not get an error.

Roalt
A: 

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.

plinth
A: 

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.

eWolf
A: 

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.

Vatine
+7  A: 

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.

Kragen