[PATCH] NoReturn Warning: Removed Warning for Unreachable Return

[David Blaikie]

On Sun Jan 05 2014 at 2:36:58 PM, Michael Bao <mike.h.bao at gmail.com> wrote:

> On Sat, Jan 4, 2014 at 1:39 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
> >
> > That makes sense as to why you'd move it, but I'm still concerned
> > about this changing the semantics. For instance, this can now fire for
> > ObjCMethodDecl objects, where it used to not be possible. This could
> > be a bug fix, but it would also require further testing (and
> > confirmation from some of the ObjC experts as to whether this is
> > desirable).
>
> Hm, wouldn't the wrap around the diagnostic call with
>
> if (const FunctionDecl *FD = getCurFunctionDecl()) {
> }
>
> prevent it from firing for ObjCMethodDecl objects?
>
> But looking at the bigger picture, it seems like Joerg was more so
> envisioning that the warning be replaced with a better warning in the
> case that the 'return' statement is unreachable. However, the default
> behavior for the unreachable code warning is to ignore it unless
> -Wunreachable-code is passed into the compiler.
>
> So in the case that the 'return' statement is reachable, we definitely
> want to emit the 'warn_noreturn_function_has_return_expr' warning.
>

Right - this should be a runtime diagnostic (ie: only fire on reachable
code). Return is reachable so warn. This should equally fire for falling
off the end of the function, of course. ( in the absence of an explicit
return)


> However, when it isn't, I think it would be best to only emit a
> warning only if -Wunreachable-code is passed into the compiler. Does
> that sound reasonable?
>

Sounds like the right thing to me. -Wunreachable-code still needs a lot of
smarts/improvements to reduce the false-positive rate & I wouldn't want to
duplicate that effort or lower the diagnostic quality in this case.

(the only 'out' here is if a noreturn function gives us a higher/better
signal for bug finding
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140106/6586063d/attachment.html>