Why "Not All Control Paths Return a Value" Is Warning and Not an Error

Warning : Not all control paths return a value c++

It's because, as the warning says, not all paths of your code return a value while the function has a distinct return type which tells the compiler "hey, I'm going to return something." but you don't actually do that if Dest is anything other than 1 or 2.


You commented:

Dest can only be 1 or 2 (it's an enum)

Yes okay but only you know that, your compiler doesn't, and it won't take your word for it. It can only see the static properties of your code, it can't predict how the runtime will go and thus it won't accept your code. For all it knows Dest can be changed by an external piece of code etc etc.


You should add some sort of default value:

int Fct_name (int nb1, int * nb2) 
{
switch (Dest)
{
case 1 :
return Fct_1(nb1,nb2);
case 2 :
return -1;
}
return 0;
}

What does warning: not all control paths return a value mean? (C++)

Your compiler isn't smart enough to take into account that the only two options for side are left and right, so it thinks it's possible for neither return statement to be executed. When side is neither left nor right, your function doesn't say which value to return.

Why not all control paths return a value is warning and not an error?

Failing to return a value from a function that has a non-void return type results in undefined behaviour, but is not a semantic error.

The reason for this, as far as I can determine, is largely historical.

C originally didn't have void and implicit int meant that most functions returned an int unless explicitly declared to return something else even if there was no intention to use the return value.

This means that a lot of functions returned an int but without explicitly setting a return value, but that was OK becase the callers would never use the return value for these functions.

Some functions did return a value, but used the implicit int because int was a suitable return type.

This means that pre-void code had lots of functions which nominally returned int but which could be declared to return void and lots of other functions that should return an int with no clear way to tell the difference. Enforcing return on all code paths of all non-void functions at any stage would break legacy code.

There is also the argument that some code paths in a function may be unreachable but this may not be easy to determine from a simple static analysis so why enforce an unnecessary return?

C++ warning: Not all control paths return a value

It requires some analysis to prove that all cases return.

And it seems your compiler doesn't do the full analysis

int Matrix::operator()(int i, int j) const
{
int _size = (_vec.size() + 2) /3;
if ((i >= _size || i < 0) || (j >= _size || j < 0)) throw OVERINDEXED;
if (i != j && j != 0 && j != _size - 1) return 0;
else {
if (j == 0)
{
return _vec[i];
}
else if (j == _size - 1)
{
return _vec[_size + i];
}
else if (i == j && j != 0 && j != _size - 1)
{
return _vec[(_size * 2) + i];
}
else
{
// No return here.
// But is this case reachable?
// yes, for (i, j) respecting:
// (0 <= i && i < _size) && (0 <= j && j < _size)
// && ((i == j) || (j == 0) || (j == _size - 1)) // #2
// && (j != 0) && (j != _size - 1) // #1
// && (i != j || j == 0 || j == _size - 1) // #3
// which after simplification results indeed in false.
// #1 simplifies #2 to (i == j) and #3 to (i != j)
}
}
}

On the other part, that means that you do useless "tests" that you can remove (and so please the compiler):

int Matrix::operator()(int i, int j) const
{
int _size = (_vec.size() + 2) /3;
if ((i >= _size || i < 0) || (j >= _size || j < 0)) throw OVERINDEXED;
if (i != j && j != 0 && j != _size - 1) return 0;
else {
if (j == 0)
{
return _vec[i];
}
else if (j == _size - 1)
{
return _vec[_size + i];
}
else // We have necessary (i == j && j != 0 && j != _size - 1)
{
return _vec[(_size * 2) + i];
}
}
}

Not All Control Paths Return a Value? Warning

Any advice?

The compiler is giving you the best advice. Not all the control paths in your function contain a return statement, and your function is supposed to return a value.

If an exception is thrown and control is transferred to a catch handler, that handler will print something to cerr and then flow off the end of your function without actually returning anything.

This is Undefined Behavior. Per Paragraph 6.6.3/2 of the C++11 Standard:

[..] Flowing off the end of a function is equivalent to a return with no value; this results in undefined
behavior in a value-returning function
.

For default-constructible values, you could fix this by adding a return T() statement right before the end of your function:

template<typename T>
T Stack<T>::Pop()
{
try
{
// ...
}
catch (OutOfBoundsException&)
{
// ...
}
catch (...)
{
// ...
}

return T();
// ^^^^^^^^^^^
}

A more reasonable approach is, however, to not have Pop() swallow the exception, but rather re-throw it. Pop() does not have strategic-level information on how to recover from an error occurred in this context:

template<typename T>
T Stack<T>::Pop()
{
try
{
// ...
}
catch (OutOfBoundsException&)
{
// ...
throw; // <== Re-throw after printing the diagnostic
}
catch (...)
{
// ...
throw; // <== Re-throw after printing the diagnostic
}
}

Even better would be if the responsibility for logging an error message did not belong to Pop() at all, since Pop() is likely supposed to be re-used by code with different requirements in this sense (some may not want to log anything, some may want to log messages to a file, some may want to log messages in a different language, and so on).

So a more reasonable version of your function would actually be:

template<typename T>
T Stack<T>::Pop()
{
if (m_index<0) throw OutOfBoundsException(m_index);
--m_index;
return(m_array[m_index]);
}

In general, you should try (no pun intended) to avoid try/catch blocks unless you have to:

  • Translate an exception
  • Recover from the error (but you need strategic knowledge for doing this)

If this not your task (as is the case for a function like Pop() above), in most situations the best thing to do is to not handle exceptions at all and let them propagate up the call stack.

To quote Dave Abrahams:

Often the best way to deal with exceptions is to not handle them at all. If you can let them pass through your code and allow destructors to handle cleanup, your code will be cleaner.

To avoid leaking memory, resources, or in general responsibilities, write code that is exception-safe by using adequate RAII wrappers. Excellent guidelines in this sense are given in this two-part talk by Jon Kalb.

In particular, avoid writing catch (...) handlers: exceptions were invented to prevent programmers from ignoring errors, and swallowing them all in a universal handler without re-throwing them is the best way to ignore them.


NOTE:

Notice, that your implementation of Pop() is a bit problematic: what happens if the copy constructor or move constructor of T throws when returning the element back to the caller, after you already modified the stack pointer?

This is the reason why the C++ Standard Library defines two separate functions pop() and top(): because it allows to offer strong guarantee, i.e. to give transactional semantics to your pop() operation - either the element is removed without exceptions being thrown, or the function had no effect at all.



Related Topics



Leave a reply



Submit