[PATCH] NoReturn Warning: Removed Warning for Unreachable Return

[Aaron Ballman]

> Index: lib/Sema/SemaStmt.cpp
> ===================================================================
> --- lib/Sema/SemaStmt.cpp
> +++ lib/Sema/SemaStmt.cpp
> @@ -2776,9 +2776,6 @@
>    QualType RelatedRetType;
>    if (const FunctionDecl *FD = getCurFunctionDecl()) {
>      FnRetType = FD->getResultType();
> -    if (FD->isNoReturn())
> -      Diag(ReturnLoc, diag::warn_noreturn_function_has_return_expr)
> -        << FD->getDeclName();
>    } else if (ObjCMethodDecl *MD = getCurMethodDecl()) {
>      FnRetType = MD->getResultType();
>      if (MD->hasRelatedResultType() && MD->getClassInterface()) {
> @@ -2953,6 +2950,12 @@
>        !CurContext->isDependentContext())
>      FunctionScopes.back()->Returns.push_back(Result);
>
> +  if (const FunctionDecl *FD = getCurFunctionDecl()) {
> +    if (FD->isNoReturn())
> +      DiagRuntimeBehavior(ReturnLoc, Result,
> +        PDiag(diag::warn_noreturn_function_has_return_expr) << FD->getDeclName());

No need to pass in getDeclName; the diagnostics engine already
understands named decls.

> +  }
> +
>    return Owned(Result);
>  }
>
> Index: test/Sema/return-noreturn.c
> ===================================================================
> --- test/Sema/return-noreturn.c
> +++ test/Sema/return-noreturn.c
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 %s -fsyntax-only -verify -fblocks -Wmissing-noreturn -Wno-unreachable-code
> +// RUN: %clang_cc1 %s -fsyntax-only -verify -fblocks -Wmissing-noreturn -Wno-unreachable-code -Winvalid-noreturn
>
>  int j;
>  void test1() { // expected-warning {{function 'test1' could be declared with attribute 'noreturn'}}
> @@ -40,3 +40,11 @@
>  _Noreturn void test5() {
>    test2_positive();
>  }
> +
> +// Should not warn about an invalid return even if the function is
> +// marked 'noreturn' and a return statement is present
> +// in the case that the return statement is unreachable.
> +__attribute__((noreturn)) void test6() {
> +  while (1) {}
> +  return;
> +}
>

Other than that nit, LGTM!

~Aaron

On Tue, Jan 14, 2014 at 11:44 AM, Michael Bao <mike.h.bao at gmail.com> wrote:
>   Ping.
>
>   Rebased off Rev 199220.
>
>   To sum it up, only emit a warning about a return statement in a noreturn function if the return statement is reachable. Otherwise, let the unreachable code warning (-Wunreachable-code) take care of it.
>
> http://llvm-reviews.chandlerc.com/D2508
>
> CHANGE SINCE LAST DIFF
>   http://llvm-reviews.chandlerc.com/D2508?vs=6343&id=6445#toc
>
> Files:
>   lib/Sema/SemaStmt.cpp
>   test/Sema/return-noreturn.c
>
> Index: lib/Sema/SemaStmt.cpp
> ===================================================================
> --- lib/Sema/SemaStmt.cpp
> +++ lib/Sema/SemaStmt.cpp
> @@ -2776,9 +2776,6 @@
>    QualType RelatedRetType;
>    if (const FunctionDecl *FD = getCurFunctionDecl()) {
>      FnRetType = FD->getResultType();
> -    if (FD->isNoReturn())
> -      Diag(ReturnLoc, diag::warn_noreturn_function_has_return_expr)
> -        << FD->getDeclName();
>    } else if (ObjCMethodDecl *MD = getCurMethodDecl()) {
>      FnRetType = MD->getResultType();
>      if (MD->hasRelatedResultType() && MD->getClassInterface()) {
> @@ -2953,6 +2950,12 @@
>        !CurContext->isDependentContext())
>      FunctionScopes.back()->Returns.push_back(Result);
>
> +  if (const FunctionDecl *FD = getCurFunctionDecl()) {
> +    if (FD->isNoReturn())
> +      DiagRuntimeBehavior(ReturnLoc, Result,
> +        PDiag(diag::warn_noreturn_function_has_return_expr) << FD->getDeclName());
> +  }
> +
>    return Owned(Result);
>  }
>
> Index: test/Sema/return-noreturn.c
> ===================================================================
> --- test/Sema/return-noreturn.c
> +++ test/Sema/return-noreturn.c
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 %s -fsyntax-only -verify -fblocks -Wmissing-noreturn -Wno-unreachable-code
> +// RUN: %clang_cc1 %s -fsyntax-only -verify -fblocks -Wmissing-noreturn -Wno-unreachable-code -Winvalid-noreturn
>
>  int j;
>  void test1() { // expected-warning {{function 'test1' could be declared with attribute 'noreturn'}}
> @@ -40,3 +40,11 @@
>  _Noreturn void test5() {
>    test2_positive();
>  }
> +
> +// Should not warn about an invalid return even if the function is
> +// marked 'noreturn' and a return statement is present
> +// in the case that the return statement is unreachable.
> +__attribute__((noreturn)) void test6() {
> +  while (1) {}
> +  return;
> +}
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>