Sign in to follow this  
load_bitmap_file

C++ - When not all paths lead to a return?

Recommended Posts

I was curious, so I compiled the following program and ran it in VS 2003:
#include <iostream>

int fn(int num)
{
	if(num > 5)
		return 7;
}

int main()
{	
	std::cout << fn(3) << "\n";

	return 0;
}

In debug mode it printed a random 5 digit negative number and in release mode it printed a random 5 digit positive number. I was surprised this even compiled, I thought that I would get an error message saying "not all paths lead to a return" or something. Yet, it does and spits out weird things. Does anyone know if this is even supposed to compile? Or is VS 2003 just being weird?

Share this post


Link to post
Share on other sites
mumpo    534
When I do that sort of thing I get a warning, but it still compiles and does the random return value thing. Don't ask me what the official answer is though.

Share this post


Link to post
Share on other sites
mutex    1111
Correct me if I'm wrong, but I believe by convention the return value is stored in the eax register on x86 architectures, and so omitting a return statement will cause the function to essentially return whatever happens to be in eax. Since it's used for all types of calculations, you're bound to get strange things returned.

Share this post


Link to post
Share on other sites
DarkMerchant    100
well if you call the function and the variable passed in is not less then 5 then you are just outputing some random value...whatever the system happens to want to return as an integer for your function call. It will always be a random chode value, just whatever is in memory at that time unless the number you pass in is greater then 5. It will compile but it will output random nothingness values.

Share this post


Link to post
Share on other sites
ScootA    270
This is a common problem. A neat way to solve this is to add this in your header files:

#pragma warning( error : 4715 )

Now whenever your paths may possibly not return a value, you will get an error instead of an easy-to-miss warning. You really don't want to let one of those slip by as they can be very tricky to debug.

Personally I think this should always be an error, even by default.

Share this post


Link to post
Share on other sites
Sheeva_    122
OK, this is really some weird results
my VS 2003 behaves in the same way, BUT only for C++. I tried similar code in C#, and the compile failed with "not all code paths return a value" error. Considering the compilers were written with the same inner rules, it seems that it's a correct C++ snippet. It's actually logical and correct, in such a piece of code

enum Answer {Yes,No};

bool func(Answer a)
{
switch(a)
{
case Yes:return true;break;
case No:return false;break;
}
}

Adding an additional return statement in the end of the function is not natural and senseless. So, C++ just gives you the choice to add the return in the endc when you need it

Share this post


Link to post
Share on other sites
antareus    576
Quote:
Original post by Sheeva_
OK, this is really some weird results
my VS 2003 behaves in the same way, BUT only for C++. I tried similar code in C#, and the compile failed with "not all code paths return a value" error. Considering the compilers were written with the same inner rules, it seems that it's a correct C++ snippet. It's actually logical and correct, in such a piece of code

Nope. In either language you can cast an arbitrary integer value to the enum type and sneak another value in.

Share this post


Link to post
Share on other sites
mutex: Your explanation for the random values sounds reasonable [smile].

ScootA: Neat trick. I haven't been bitten by this yet, but chances are I will sometime in the future. [grin]

Does anyone know whether having the possibility that the function does not return a value is allowed in the C++ standard or not? Or is this another one of those undefined behavior things? It seems kinda weird to allow this sort of thing to happen.

Share this post


Link to post
Share on other sites
nprz    692
You are lucky compilers add a return statement at the end of functions at all. Otherwise your code would continue to execute whatever code follows that function.

I don't know what the C/C++ standard on this is, but just imagine what might have happened if the compiler didn't return at all [grin]

Share this post


Link to post
Share on other sites
Sheeva_    122
It shouldn't be, and here's the reason: when you return a random int, it's usually not really dangerous. But when the function returns something that contains pointers, and the pointers are random, this leads to totally weird results, like data corruption. Even with the int case, if the function is supposed to return an array index, the bug could be quite hard to spot out without boundschecking. And there are lots of other errors with random return values. A random bool will be true in almost all of the cases, for instance. It's not so obvious that it is random as an integer.
So every such function is a potential bug, even if you're sure it never happens (because you're never 100% sure). I would consider this as an error if I developed the standard :)

Share this post


Link to post
Share on other sites
Sheeva_    122
Quote:
You are lucky compilers add a return statement at the end of functions at all. Otherwise your code would continue to execute whatever code follows that function.

I don't know what the C/C++ standard on this is, but just imagine what might have happened if the compiler didn't return at all


Yeah, like the good ol' assembler days [sick]

Share this post


Link to post
Share on other sites
Washu    7829
First of all, you should be getting a warning. Secondly, in debug mode it will initialize registers and memory to very specific values. Most likely 0xCCCCCCCC or 0xCDCDCDCD, although there are a few other values it uses as well.

This doesn't happen in release mode, and hence whatever is in eax is what you get. Using the pragma to convert the warning to an error is recommended.

Sheeva: the rules are vastly different. C++ is a much more complex language than C#, and C# is much stricter in it's application of it's rules. What might constitute "implementation specified" behavior in C++ constitutes erroronious behavior in C#. Just because they both start with a C doesn't mean they behave the same.

As for your switch statement, what if the value isn't Yes or No? You do realize that you can cast an arbitrary integer to an enumeration, and it is perfectly legal for the value to not be in the range of those provided by the enumeration. For instance, your code would break on this func((Answer)2);. However, that is a perfectly legal call. Thus not all paths DO return a value, hence you either need to insert a default case that throws (the best idea) or just add an additional return statement.

Share this post


Link to post
Share on other sites
Sheeva_    122
Quote:
Sheeva: the rules are vastly different. C++ is a much more complex language than C#, and C# is much stricter in it's application of it's rules. What might constitute "implementation specified" behavior in C++ constitutes erroronious behavior in C#. Just because they both start with a C doesn't mean they behave the same.

That's not what I meant. What I meant is that C++ .NET and C# are (or at least could) be used together in managed applications, and so their behavior should be at least similar to my mind.
Quote:
As for your switch statement, what if the value isn't Yes or No?

OK, OK, that was a stupid example. I wanted to show the choice of some known values (or states, or anything), when we do handle all of the possible values.
Quote:
You do realize that you can cast an arbitrary integer to an enumeration

Yeah, C++ allows you to cope with lots of things that are logically wrong, like such casting to yes/no. Take the pointer juggling to get access to private members of a class for instance. That doesn't mean you must do each and every function so foolproof.
Quote:
you either need to insert a default case that throws (the best idea) or just add an additional return statement.

Since the problem is probably logical, and you shouldn't go and cast random ints to enums around, it isn't necessary to throw, and the additional return statement is pointless, too (in this particular case)

Share this post


Link to post
Share on other sites
Washu    7829
Quote:
Original post by Sheeva_
Quote:
Sheeva: the rules are vastly different. C++ is a much more complex language than C#, and C# is much stricter in it's application of it's rules. What might constitute "implementation specified" behavior in C++ constitutes erroronious behavior in C#. Just because they both start with a C doesn't mean they behave the same.

That's not what I meant. What I meant is that C++ .NET and C# are (or at least could) be used together in managed applications, and so their behavior should be at least similar to my mind.

While C++ can interact with managed languages via the managed extentions, that doesn't mean that all of it's features are available in the other managed languages. Indeed, they are fairly restrictive in the behaviors allowed in the others purely because they do NOT interop directly with unmanaged code.
Quote:

Quote:
As for your switch statement, what if the value isn't Yes or No?

OK, OK, that was a stupid example. I wanted to show the choice of some known values (or states, or anything), when we do handle all of the possible values.
Quote:
You do realize that you can cast an arbitrary integer to an enumeration

Yeah, C++ allows you to cope with lots of things that are logically wrong, like such casting to yes/no. Take the pointer juggling to get access to private members of a class for instance. That doesn't mean you must do each and every function so foolproof.
No, I mean in C#. Not just C++.
Quote:

Quote:
you either need to insert a default case that throws (the best idea) or just add an additional return statement.

Since the problem is probably logical, and you shouldn't go and cast random ints to enums around, it isn't necessary to throw, and the additional return statement is pointless, too (in this particular case)

Actually, the behavior is exceptional. There are many cases where you might want to cast an integer to an enumeration, but if the programmer makes a mistake, an exception will tell you. Something that silently ignoring it will not. Ignoring it can easily result in the bug showing up elsewhere, instead of much earlier and closer to the source. Hence you SHOULD throw.

Share this post


Link to post
Share on other sites
Andrew Russell    1394
Tidbit:

The fact that not returning a value is only a warning (not an error) in C++, is because there used to be no "void" type in C. Thus "int" was used instead. It must remain a warning for compatability with old C code.

C#, not requiring backwards compatability with old, crap languages, does not have this problem.

Share this post


Link to post
Share on other sites
Andrew Russell    1394
Quote:
Something that silently ignoring it will not. Ignoring it can easily result in the bug showing up elsewhere, instead of much earlier and closer to the source. Hence you SHOULD throw.

Being programmer error, an assert() may be more appropriate.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster   
Guest Anonymous Poster
Ugh, don't use #pragma

Use the proper command line switches to treat all warnings as errors and write clean code (with cl.exe, /Wall /WX). Better yet, use gcc as a lint checker of your code. Gcc complains about 10x more stuff than Visual Studio in maximum warning mode (in my professional work, tons of things that are acceptable by vc++ are not acceptable by gcc, but if it was acceptable by gcc, vc++ was always still happy. This was with VC++ 7.1 and GCC 3.3.3).

If you must disable specific warnings, disable them on the invocation of your compiler (i.e. cl.exe can use /wd to disable a specific warning on the command line).

If you do it this way, you won't litter your source code with wierd build directives which are more often than not compiler specific.

Share this post


Link to post
Share on other sites
Washu    7829
Quote:
Original post by Andrew Russell
Quote:
Something that silently ignoring it will not. Ignoring it can easily result in the bug showing up elsewhere, instead of much earlier and closer to the source. Hence you SHOULD throw.

Being programmer error, an assert() may be more appropriate.


I should have clarified, it may or may not be a programming error. For instance, data read from a configuration file or other source. You cannot guarantee that the data is valid, so you throw an exception instead.

Share this post


Link to post
Share on other sites
iMalc    2466
I too have gotten used to using 'treat warnings as errors" and I must say that I actually find it a very nice way of coding.

It is entirely possible to write code where logically all return paths to return a value, yet the compiler still gives a warning because it looks like it can get to the end where there is no return. This can never be solved fully because it's essentially the same thing as the halting problem.
Sheeva's example is not one of these cases though as has been already pointed out.

However in those cases I prefer to simply give in and keep the compiler happy.

Share this post


Link to post
Share on other sites
Sneftel    1788
It bears mentioning that the standard specifies that a non-void function that completes without a return statement produces "undefined behavior", not "an undefined value". In other words, it might corrupt the stack, crash the program, crash the computer, or treat your dog with disrespect. So even if you expect to ignore the return value, it isn't a legitimate trick.

Share this post


Link to post
Share on other sites
Jiia    592
Quote:
Original post by iMalc
It is entirely possible to write code where logically all return paths to return a value, yet the compiler still gives a warning because it looks like it can get to the end where there is no return.

I agree. And including a return value that will never execute is a bit confusing to anyone skimming over the code. The proper way to handle this is to put something such as this at the very bottom:

return HardcoreFatalError("Cows are flying");

Share this post


Link to post
Share on other sites
Nitage    1107
Quote:
Original post by Washu
Quote:
Original post by Andrew Russell
Quote:
Something that silently ignoring it will not. Ignoring it can easily result in the bug showing up elsewhere, instead of much earlier and closer to the source. Hence you SHOULD throw.

Being programmer error, an assert() may be more appropriate.


I should have clarified, it may or may not be a programming error. For instance, data read from a configuration file or other source. You cannot guarantee that the data is valid, so you throw an exception instead.


The choice between an using an Assert and thowing an exception should be made depending on the contex of the function involved.

Quote:

return HardcoreFatalError("Cows are flying");

Pigs surely?

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this