This RFC proposes making the -Wreturn-type warning an error by default (i.e. it can still be downgraded to a warning by passing -Wno-error=return-type).
Background
Falling off the end of a function is UB in C++ and C (though in the latter case only if the return value is actually used), in contrast to nearly every other modern ahead-of-time compiled language where this is a hard error. The status quo in C and C++ isn’t exactly desirable: there is rarely a case where actually falling off the end of a function that is supposed to return a value is desired behaviour. If that ever does happen, it tends to be a horrible bug that is difficult to track down.
The problem of false positives
Unfortunately however, it seems that the solution to this issue isn’t as simple as ‘just making it an error’. The main problem is that the CFG analysis required to determine whether a function returns on all control paths is far from perfect. Consider:
int f(int x) {
if (x == 1) return 1;
if (x != 1) return 2;
}
If you examine this function carefully, it should be clear that there can be no value of x for which f doesn’t return. Clang, however, doesn’t know that and issues a -Wreturn-type warning regardless.
Fixing this and similar cases is at best non-trivial and at worst impossible: reducing false positives in a systematic manner would require implementing what would essentially boil down to some strange version of InstCombine in the frontend, which is something that in my opinion is neither feasible nor desirable—both because I doubt anyone here wants to implement and maintain something like that for a single warning, and because doing this in the frontend kind of defeats the point of having an IR-based compiler framework; maybe a CIR-based solution will be able to ameliorate this situation at some distant point in the future by deferring this sort of analysis to a later compilation stage, after some amount of folding has already taken place, but for now, this categorically just isn’t viable, and I’m sure it would come with its own problems. In any case, the fact remains that what we’re trying to determine here is whether an arbitrary function will always return given arbitrary input. At the end of the day, we just end up at the halting problem.
In other words, resolving all false positives for this is impossible, and resolving any non-trivial false positives is infeasible. So we’re stuck with the false positives, and any discussion around making this warning an error (by default) needs to account for the fact that Clang will reject semantically valid programs.
Should we care about false positives?
At the risk of sounding a bit dismissive—and I doubt this really needs to be pointed out—but this isn’t a problem that’s specific to Clang, or C, or C++. Almost every other statically-compiled high-level language necessarily suffers from the same problem, and typically treat the equivalent of our function f above as a hard error (there are some exceptions: e.g. Ada only issues a warning, and Fortran doesn’t care at all).
That is, they all run into the same false positives, and they all seem to agree that allowing one to not return a value from a function is not defensible in a modern programming language. The fact that this is at least a warning in Clang which is enabled by default and that there are many people who use assert(false), __builtin_unreachable(), and similar facilities to convince the compiler that some code path is, in fact, unreachable rather than turning off the warning is a pretty good indication that falling off the end of a function is categorically a bug. Really, the only reason this isn’t an error by default is for historical reasons: if anyone was to design C or C++ today, I doubt they wouldn’t make this a hard error.
Now, of course, it’s easy to say that ‘anyone who writes code like our f above is just doing it wrong; they should just fix their code’, but making something the user’s problem and breaking everyone’s code is not something that we like doing, and for good reason. While you can argue that many (if not most) cases of falling off the end of a function are probably a bug, there may well be cases out there that are genuine false positives in code bases that have been well tested and which are known to work, and the only thing we’d be doing to those is suddenly break their code ‘for no reason’—at least from their point of view.
The main problem here, then, is what do we do about all the existing code that nobody touches but just compiles over and over again. If this change truly does start breaking code that simply cannot be updated or even have its compiler flags changed, then that would really make this change untenable. The question is whether any such code actually exists or whether these are hypotheticals that we’re worrying about for nothing, and I’m not sure there is a good way of finding this out other than trying to make the change and see if anyone starts complaining about it—unless anyone here happens to know a code base that would be affected by this.
A possible compromise, then, would be to only make this an error in newer language modes (e.g. C++23/26 and perhaps C23/2y); this is more likely to have the desired effect (viz. new code benefits from this being an error, and old code isn’t affected). While it is still possible to argue that this might impede the adoption of e.g. C++23 in an affected code base, I should hope that anyone who is updating their code base to compile under a modern standard such as C++23 is also willing to update their code proper if it stops compiling as a result—or at the very least, if they’re already changing one compiler flag, they can at the very least also add another and pass -Wno-error=return-type as a last resort.
Addendum: switch statements
A special case that is worth mentioning here is:
enum E { A, B };
int g(E e) {
switch (e) {
case A: return 1;
case B: return 2;
}
}
I’m bringing up this case because this is actually one that other languages with more strongly-typed enums or discriminated unions tend to accept (via exhaustive pattern matching), because they generally don’t let you cast any random number to an enum type. Unlike the perhaps a bit contrived f above, this is actually a fairly reasonable pattern. One option here is of course to just put e.g. __builtin_unreachable() or assert(false) or something similar after the switch statement, but as it turns out, Clang actually doesn’t diagnose this either—not even if you pass -Wall -Wextra -Werror=return-type. There are additional flags you can pass to make us diagnose this (which you might want to enable if casting to enums is commonplace in your code base), but this is an exception where we have a pattern that is reasonably common to where it makes sense to consider this well-formed (because it is, assuming no-one ever casts anything to E).
Thus, notwithstanding everything that was pointed out earlier, we can definitely add support to the CFG for common patterns such as this one whose ill-formedness is more contentious—it’s just that we can’t support all possible patterns, and there will always be false positives. And while you can at least make a good case for this g here, our f above is not so much of a ‘pattern’ but really just an antipattern that isn’t worth supporting: the if (x != 1) is tautological and thus useless; you can just remove it and write either else return 2; or just return 2;, both of which we don’t warn about.
Conclusion
Defaulting -Wreturn-type to an error has its share of problems, but I’d argue that it is worth pursuing. I can think of a few possible ways in which we could approach this (see below).
We should also decide whether we want to treat C and C++ differently here—not just because falling off the end of a function isn’t always UB in C, but also because C++ devs tend to be more accepting of their code breaking all the time than C devs.
Finally, thanks also a lot to @AaronBallman for helping me w/ this RFC!
Possible approaches
- Option 1: Make this an error by default in all language modes.
- Option 2: Make this an error by default starting in some reasonably modern language mode (we can decide on one later, but one of C++20/23/26 and C23/2y seems reasonable to me).
- Option 3: Make this an error in
clangdso only people writing new code are told to ‘do better’ more aggressively and revisit this discussion in a few years. - Option 4: Do nothing.
valid but I do believe if we are going to make any progress in getting to a safer language, the implementations are the places were will need to do these trials and take some risk. I do think we need to be cautious but we also need a bit of bravery here as well. We have the option of finding a middle road that gets a closer while giving folks the time to migrate their code.