False positive Wreturn-type warning when a function either returns or throws

[TiborGY]

Clang does not appear to be able to deduce that a function that always throws is effectively [[noreturn]], not even in seemingly trivial cases. Consider this:

void throwError(const std::string& msg){
    throw std::runtime_error(msg);
}

int ensureZero(const int i){
    if (i == 0) return 0;
    throwError("ERROR");
}

Compiling with -Wall -Wextra -O3 -std=c++2c yields a false positive warning:
warning: non-void function does not return a value in all control paths [-Wreturn-type]

PS. https://godbolt.org/z/hdTb6bGcr

[snarang181]

Hi, I have a PR up for this #145166

[alexfh]

I think, it's a problem in the user code that should be solved outside of the compiler. I'd expect throwError to be annotated with the [[noreturn]] attribute, which would allow clang to correctly reason about code paths in ensureZero: https://gcc.godbolt.org/z/Esd7xjfPn

Manually annotating the throwError function with [[noreturn]] would work when the definition of the function is not visible (i.e. it's defined in a different translation unit): https://gcc.godbolt.org/z/ha8xq1Gz5

[TiborGY]

I think, it's a problem in the user code that should be solved outside of the compiler. I'd expect throwError to be annotated with the [[noreturn]] attribute, which would allow clang to correctly reason about code paths in ensureZero: https://gcc.godbolt.org/z/Esd7xjfPn

Manually annotating the throwError function with [[noreturn]] would work when the definition of the function is not visible (i.e. it's defined in a different translation unit): https://gcc.godbolt.org/z/ha8xq1Gz5

I see your point, and it is not unreasonable, but I would expect that a state-of-the-art compiler to be able to deduce [[noreturn]] on it own (at least for simple cases like this) at high optimization levels. Please correct me if I am mistaken, but I think the "as if" rule would make this completely legal.

But you are of course completely right in that multiple translation units can easily throw a wrench in static analysis. Given this I think there are actually two (not mutually exclusive) paths to take here:

1. Weaken the phrasing of the -Wreturn-type message to acknowledge the limitations of the current static analysis. The current message leaves no room for uncertainty, it flat out says that there are control paths that hit the end of ensureZero, which is simply not true. I would propose rewording it as something like "warning: cannot prove that control does not reach end of non-void function [-Wreturn-type]". This would remain true even if and when the static analysis is improved.
2. Gradually improve static analysis. Clang already does a lot of static analysis for optimization and warnings, I dont see how this would not fit in that general trend. Simple ones can be enabled always, more expensive ones can be restricted to higher optimization levels, resulting in fewer -Wreturn-type warnings at higher optimization levels. At, say, -O3 -flto this could perhaps even extend between translation units.

[TiborGY]

PS. I personally dislike manually adding attributes when the compiler could reasonably do without them. C++ syntax is already verbose enough as it is, if one cares about const correctness, adding noexcept where appropriate, adding constexpr where possible...the keywords/attributes pile up fast.
For external APIs, I agree it is good to let the users of a library know, but in general I am against manual [[noreturn]].