[PATCH] Warnings for NoReturn Functions

[Michael Bao]

Hey Aaron,

I went ahead and made the two patches with the tests, let me know how they
look! Is there anything I need to do to close D2506?
http://llvm-reviews.chandlerc.com/D2507
http://llvm-reviews.chandlerc.com/D2508

Thanks.


On Fri, Jan 3, 2014 at 12:33 PM, Aaron Ballman <aaron at aaronballman.com>wrote:

> On Fri, Jan 3, 2014 at 12:25 PM, Michael Bao <mike.h.bao at gmail.com> wrote:
> > In response to Bug 10642 in Bugzilla (
> http://llvm.org/bugs/show_bug.cgi?id=10642). I've checked this against
> Revision 198291.
> >
> >   #  Removed the warning about a 'return' statement in a 'noreturn'
> function if the 'return' statement is unreachable.
> >   #  Warn users if they have a non-void return type on a 'noreturn'
> function.
> >
> > This is my first patch, let me know if there's anything I need to do :)
> Thanks!
>
> Thanks for your submission!
>
> You should reformat the patch to meet the coding guidelines:
> http://www.llvm.org/docs/CodingStandards.html  For instance, spaces
> instead of tabs, no braces around single-line compound statements,
> etc.
>
> Also, this should ideally be split into two patches since there are
> two different things going on here.
>
> You should have test cases that demonstrate the behavioral changes for
> both patches.
>
> > http://llvm-reviews.chandlerc.com/D2506
> >
> > Files:
> >   lib/Sema/SemaStmt.cpp
> >   lib/Sema/SemaDecl.cpp
> >   include/clang/Basic/DiagnosticSemaKinds.td
> >
> > Index: lib/Sema/SemaStmt.cpp
> > ===================================================================
> > --- lib/Sema/SemaStmt.cpp
> > +++ lib/Sema/SemaStmt.cpp
> > @@ -2776,9 +2776,10 @@
> >    QualType RelatedRetType;
> >    if (const FunctionDecl *FD = getCurFunctionDecl()) {
> >      FnRetType = FD->getResultType();
> > -    if (FD->isNoReturn())
> > -      Diag(ReturnLoc, diag::warn_noreturn_function_has_return_expr)
> > -        << FD->getDeclName();
> > +    if (FD->isNoReturn()) {
> > +                       DiagRuntimeBehavior(ReturnLoc, RetValExp,
> > +
> PDiag(diag::warn_noreturn_function_has_return_expr) << FD->getDeclName());
>
> You can leave off the getDeclName here -- the diagnostics engine
> understands anything that's a NamedDecl.
>
> > +               }
> >    } else if (ObjCMethodDecl *MD = getCurMethodDecl()) {
> >      FnRetType = MD->getResultType();
> >      if (MD->hasRelatedResultType() && MD->getClassInterface()) {
> > Index: lib/Sema/SemaDecl.cpp
> > ===================================================================
> > --- lib/Sema/SemaDecl.cpp
> > +++ lib/Sema/SemaDecl.cpp
> > @@ -7392,6 +7392,10 @@
> >      AddToScope = false;
> >    }
> >
> > +       if (NewFD->isNoReturn() &&
> !NewFD->getResultType()->isVoidType()) {
> > +               Diag(NewFD->getLocation(),
> diag::warn_noreturn_function_has_nonvoid_type)
> > +                       << NewFD->getDeclName();
>
> You can also leave off the getDeclName here.
>
> > +       }
> >    return NewFD;
> >  }
> >
> > Index: include/clang/Basic/DiagnosticSemaKinds.td
> > ===================================================================
> > --- include/clang/Basic/DiagnosticSemaKinds.td
> > +++ include/clang/Basic/DiagnosticSemaKinds.td
> > @@ -6462,6 +6462,9 @@
> >  def warn_falloff_noreturn_function : Warning<
> >    "function declared 'noreturn' should not return">,
> >    InGroup<InvalidNoreturn>;
> > +def warn_noreturn_function_has_nonvoid_type : Warning<
> > +       "function %0 declared 'noreturn' should have not have a non-void
> return type">,
> > +       InGroup<InvalidNoreturn>;
>
> This may be a bit more of a nitpick than is strictly necessary, but
> 'noreturn' is the C++11 spelling, and _Noreturn is the C11 spelling --
> we should probably be a bit more distinct with the diagnostic for
> clarity purposes.
>
> >  def err_noreturn_block_has_return_expr : Error<
> >    "block declared 'noreturn' should not return">;
> >  def err_noreturn_missing_on_first_decl : Error<
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
>
> Thanks!
>
> ~Aaron
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140103/941e7f7e/attachment.html>