views:

415

answers:

7

I have a few config options in my application along the lines of

const bool ExecuteThis=true;
const bool ExecuteThat=false;

and then code that uses it like

if(ExecuteThis){ DoThis(); }
if(ExecuteThat){ DoThat(); } //unreachable code warning here

The thing is, we may make slightly different releases and not ExecuteThis or ExecuteThat and we want to be able to use consts so that we don't have any speed penalties from such things at run time. But I am tired of seeing warnings about unreachable code. I'm a person that likes to eliminate all of the warnings, but I can't do anything about these. Is there some option I can use to turn just these warnings off?

+10  A: 

To disable:

#pragma warning disable 0162

To restore:

#pragma warning restore 0162

For more on #pragma warning, see MSDN.

Please note that the C# compiler is optimized enough to not emit unreachable code. This is called dead code elimination and it is one of the few optimizations that the C# compiler performs.

And you shouldn't willy-nilly disable the warnings. The warnings are a symptom of a problem. Please see this answer.

Jason
Please be sure to read Lasse V. Karlsen's (http://stackoverflow.com/users/267/lasse-v-karlsen) answer (http://stackoverflow.com/questions/1930793/is-there-a-way-to-get-vs2008-to-stop-warning-me-about-unreachable-code/1930840#1930840). All I did is tell you how to do what you wanted. His answer tells you why you should avoid being in this situation in the first place.
Jason
I agree with this, with the caveat that you should comment in why you are choosing not to use the precompiler. I am guessing it is so you can use the compiler to statically check for errors in the code, even if it isn't being compiled into your app in the current build.I wonder if the C# compiler is smart enough to "optimize" this code out, so that you end up with the same code as if you had used the pre-compiler.
Merlyn Morgan-Graham
IMNO you shouldn't deactivate the compiler warnings, you should always be informed of the "bad", or "dead" code you write, even if you are conscient of it. The preprocessed statements (#if condition ) could help you in your case.
serhio
+13  A: 

What about using preprocessor statements instead?

#if ExecuteThis
    DoThis();
#endif

#if ExecuteThat
    DoThat();
#endif
Jason Down
+1. That way the code you aren't using doesn't even get compiled into the assembly.
David
I like this method, but I value the fact that my code must always compile no matter what the preprocessor directives are..
Earlz
Preprocessor statements are definately the way to set flags at compile-time, and will ensure the "unreachable code" won't be compiled, much more safely than a const bool. Plus, you can make extra configurations besides just "Debug" and "Release" that will insert these defines, such as "Debug ExecuteThis".
Will Eddins
I so wish I could have two answers :) I think I'll be following a hybrid approach...
Earlz
+1 you wrote the same as me but faster :)
slugster
They're giving you warnings for a reason, because this is the way it should be done.
Will Eddins
Note that using "if (x) { ... }", where x is defined as `const Boolean x = false;` will in fact not put the code into the assembly. The warning is there to tell you that that just happened, and that might be a problem.
Lasse V. Karlsen
I disagree with the notion that "this is the way it should be done." I have seen enough code to know that there is no right way. However, there are plenty of ways that are *probably* wrong. That the compiler warns me about those and thus forces me to take a stand: Use this code, or change it, is a good thing. However, sometimes the right choice is the wrong one (if you know what I mean, then you know what I mean.)
Lasse V. Karlsen
+8  A: 

Well, #pragma, but that is a but grungy. I wonder if ConditionalAttribute would be better - i.e.

[Conditional("SOME_KEY")]
void DoThis() {...}
[Conditional("SOME_OTHER_KEY")]
void DoThis() {...}

Now calls to DoThis / DoThat are only included if SOME_KEY or SOME_OTHER_KEY are defined as symbols in the build ("conditional compilation symbols"). It also means you can switch between them by changing the configuration and defining different symbols in each.

Marc Gravell
+1 One can also resort to auto-generated code if things get complex.
Hamish Grubijan
99.9k - quick! ask something profound!
x0n
No good for another few hours... maxed out much earlier...
Marc Gravell
I can't even begin to express how much I *hat*^H^H^H don't like "Conditional". Sure, it's nice and it does exactly what it is supposed to, but it defines *when* it is supposed to do something *somewhere else* than where I want it to do it. When I read the code that says "if (x) DoThis();", I expect that method to be called, not to be masked out silently. If there is a chance that this method call is going to be masked out, I want that to be explicitly expressed at the call-site, not with the method.
Lasse V. Karlsen
I guess it depends a bit on what the method is. I'm OK with trace/log/debug etc steps silently removing themselves.
Marc Gravell
+4  A: 

The easiest way is to stop writing unreachable code :D #DontDoThat

BenAlabaster
+2  A: 

The fact that you have the constants declared in code tells me that you are recompiling your code with each release you do, you are not using "contants" sourced from your config file.

So the solution is simple: - set the "constants" (flags) from values stored in your config file - use conditional compilation to control what is compiled, like this:

#define ExecuteThis
//#define ExecuteThat

public void myFunction() {
#if ExecuteThis
    DoThis();
#endif
#if ExecuteThat
    DoThat();
#endif
}

Then when you recompile you just uncomment the correct #define statement to get the right bit of code compiled. There are one or two other ways to declare your conditional compilation flags, but this just gives you an example and somewhere to start.

slugster
+1 to you for giving more detail than me ;)
Jason Down
+16  A: 

First of all, I agree with you, you need to get rid of all warnings. Every little warning you get, get rid of it, by fixing the problem.

Before I go on with what, on re-read, amounts to what looks like a rant, let me emphasis that there doesn't appear to be any performance penalty to using code like this. Having used Reflector to examine code, it appears code that is "flagged" as unreachable isn't actually placed into the output assembly.

It is, however, checked by the compiler. This alone might be a good enough reason to disregard my rant.

In other words, the net effect of getting rid of that warning is just that, you get rid of the warning.

Also note that this answer is an opinion. You might not agree with my opinion, and want to use #pragma to mask out the warning message, but at least have an informed opinion about what that does. If you do, who cares what I think.

Having said that, why are you writing code that won't be reached?

Are you using consts instead of "defines"?

A warning is not an error. It's a note, for you, to go analyze that piece of code and figure out if you did the right thing. Usually, you haven't. In the case of your particular example, you're purposely compiling code that will, for your particular configuration, never execute.

Why is the code even there? It will never execute.

Are you confused about what the word "constant" actually means? A constant means "this will never change, ever, and if you think it will, it's not a constant". That's what a constant is. It won't, and can't, and shouldn't, change. Ever.

The compiler knows this, and will tell you that you have code, that due to a constant, will never, ever, be executed. This is usually an error.

Is that constant going to change? If it is, it's obviously not a constant, but something that depends on the output type (Debug, Release), and it's a "#define" type of thing, so remove it, and use that mechanism instead. This makes it clearer, to people reading your code, what this particular code depends on. Visual Studio will also helpfully gray out the code if you've selected an output mode that doesn't set the define, so the code will not compile. This is what the compiler definitions was made to handle.

On the other hand, if the constant isn't going to change, ever, for any reason, remove the code, you're not going to need it.

In any case, don't fall prey to the easy fix to just disable that warning for that piece of code, that's like taking aspirin to "fix" your back ache problems. It's a short-term fix, but it masks the problem. Fix the underlying problem instead.

To finish this answer, I'm wondering if there isn't an altogether different solution to your problem.

Often, when I see code that has the warning "unreachable code detected", they fall into one of the following categories:

  1. Wrong (in my opinion) usage of const versus a compiler #define, where you basically say to the compiler: "This code, please compile it, even when I know it will not be used.".
  2. Wrong, as in, just plain wrong, like a switch-case which has a case-block that contains both a throw + a break.
  3. Leftover code from previous iterations, where you've just short-circuited a method by adding a return at some point, not deleting (or even commenting out) the code that follows.
  4. Code that depends on some configuration setting (ie. only valid during Debug-builds).

If the code you have doesn't fall under any of the above settings, what is the specific case where your constant will change? Knowing that might give us better ways to answer your question on how to handle it.

Lasse V. Karlsen
Outstanding. Plus one all day.
Jason
+1... beat him with that big stick :)
slugster
"It is, however, checked by the compiler. This alone might be a good enough reason to disregard my rant." -- I would say thats my reason for disregarding it... I hate having some configurations that may or may not compile.. I like to have warning-less code(as in, I fix them all) but I know that the cause of the warning is really nothing significant... (also, it's not like I'm disabling the warning for my whole project)
Earlz
+1  A: 

The quickest way to "Just get rid of it" without modifying your code would be to use

#pragma warning disable 0162

On your Namespace, class or method where you want to supress the warning.

For example, this wont throw the warning anymore:

#pragma warning disable 0162
namespace ConsoleApplication4
{
  public class Program
  {
    public const bool something = false;

    static void Main(string[] args)
    {
        if (something) { Console.WriteLine(" Not something" ); }

    } 
 }

However be warn that NO METHOD inside this namespace will throw the warning again... and well.. warnings are there for a reason (what if it happened when you did NOT planned it to be unreachable?)

I guess a safer way would be to write the variables in a configuration file, and read them from there at the beginning of the program, that way you don't even need to recompile to have your different versions/releases! Just change the app file and go :D.

about the speed penalty.. yes.. making it this way would inquire in a speed penalty... compared to using const but unless you are really worried about wating 1/100 of a millisecond more.. I would go for it that way.

Francisco Noriega