5

I have the following function (reduced example):

QByteArray DecompressBytes(const QByteArray& content){
  /* function body (with other return expressions) */

  do { return content; } while(content.size() != 0);
}

Last line was added for testing, replacing a macro used. Visual Studio doesn't see a problem with this code, but g++ generates

warning: control reaches end of non-void function [-Wreturn-type]

Changing last line to return content; removes the warning.
My question: why the compiler behaves this way and what should be the form of the code to avoid any warnings?

The last line used to be ASSERT_FAIL("must be unreachable", content) with ASSERT_FAIL expanding into do { ... } while(false) pattern with another macro replacing while to forbid non-bool expressions, so the resulting expression was like do { qt_assert_x("", "", 42); return content; } while(::helper::bool_verify(false));.

g++ version 5.3.0, used with MinGW (standard Qt setup).

Upd: after trying to comment different parts of code (since pure example above doesn't allow to reproduce the problem), something really strange emerged:

QByteArray DecompressBytes(const QByteArray& content){
  QByteArray decompressed; //no 'unused variable' warning
  do { return content; } while(content.size() != 0);
} //produces warning above

QByteArray DecompressBytes2(const QByteArray& content){
  //QByteArray decompressed;
  do { return content; } while(content.size() != 0);
} //doesn't produce warning

std::vector<char> DecompressBytes3(const std::vector<char>& content){
  std::vector<char> decompressed; //no 'unused variable' warning

  do { return content; } while(content.size() != 0);
} //does produce warning

std::vector<char> DecompressBytes4(const std::vector<char>& content){
  int decompressed; //unused variable warning is given

  do { return content; } while(content.size() != 0);
} //doesn't produce warning

Not sure what all of this means.

gsamaras
  • 71,951
  • 46
  • 188
  • 305
Abstraction
  • 1,108
  • 11
  • 25
  • 1
    I punted an answer, but on closer inspection of your example, I'm amazed the g++ issues a warning in this instance. – Bathsheba Mar 17 '17 at 14:01
  • 3
    Actually it should raise another warning, that `content.size() != 0` is not reachable. So this code is still broken. – Slava Mar 17 '17 at 14:04
  • It might be that gcc does not have a special case in it's flow control analysis for do..while. The condition might be false, so maybe the loop wont run... at-least that's valid reasoning for other loops. – sp2danny Mar 17 '17 at 14:06
  • @Slava I believe I already explained where that non-constant expression in `while()` came from. Also, I don't remember ever seeing 'unreachable code' warning with this idiom. – Abstraction Mar 17 '17 at 14:12
  • I cannot reproduce the warning with clang. Same with gcc 4.9.2, but I am using an `std::vector` instead of the QT array, since I haven't installed it. – gsamaras Mar 17 '17 at 14:19
  • For what it's worth... neither g++ 6.3.1 nor clang++ 3.9.1 (with -Wall -Werror) complain about the code as posted (regardless of optimization level specified). – G.M. Mar 17 '17 at 14:21
  • 1
    @Abstraction your explanation does not make that code not broken. Also I said it should raise that warning, but it is not obligated to do that. – Slava Mar 17 '17 at 15:47

1 Answers1

4

That's strange. GCC 4.9.2 with Wall flag doesn't complain about this similar code (I haven't qt installed)

#include <vector>

std::vector<int> foo(const std::vector<int>& v)
{
    do { return v; } while( v.size() != 0 );
}

int main() {std::vector<int> v; foo(v);}

Same with clang 4.2.1. As a result I would say that this is something specific to your installation, and if I were you I would add a dummy return statement at the end of the function, or use a flag to get rid of this warning.

Update: Reproduced the behavior of the last two new functions provided by the OP with GCC, while clang raised an 'unused variable' only for DecompressBytes4().

One possible explanation is that this you have there is not really used in practice. Dead-code elimination occurs, leaving only the return statement there.

However, when we use an extra variable the dead code elimination mechanism doesn't work in the same way, some information is preserved and it fails to see the return statement.

Usually dead code elimination happens on the back-end, while warnings are raised by the front-end. In this case it seems that these two do not communicate brilliantly! =)

PS - Dead code elimination explains why you don't get an unreachable warning too.

gsamaras
  • 71,951
  • 46
  • 188
  • 305