[PATCH] NoReturn Warning: Non-Void Return Type

[Aaron Ballman]

On Fri, Jan 3, 2014 at 2:17 PM, Michael Bao <mike.h.bao at gmail.com> wrote:
> From Bug 10642 (http://llvm.org/bugs/show_bug.cgi?id=10642).
>
> Added a warning if user provides a non-void return type for a function marked 'noreturn.'
>
> http://llvm-reviews.chandlerc.com/D2507
>
> Files:
>   lib/Sema/SemaDecl.cpp
>   include/clang/Basic/DiagnosticSemaKinds.td
>   test/Sema/warn-noreturn-return-type.c
>
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -7392,6 +7392,9 @@
>      AddToScope = false;
>    }
>
> +  if (NewFD->isNoReturn() && !NewFD->getResultType()->isVoidType())
> +    Diag(NewFD->getLocation(), diag::warn_noreturn_function_has_nonvoid_type)
> +      << NewFD->getDeclName();

No need to use getDeclName; can just pass in NewFD directly.

>    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 not have a non-void return type">,
> +  InGroup<InvalidNoreturn>;
>  def err_noreturn_block_has_return_expr : Error<
>    "block declared 'noreturn' should not return">;
>  def err_noreturn_missing_on_first_decl : Error<
> Index: test/Sema/warn-noreturn-return-type.c
> ===================================================================
> --- test/Sema/warn-noreturn-return-type.c
> +++ test/Sema/warn-noreturn-return-type.c
> @@ -0,0 +1,15 @@
> +// RUN: %clang_cc1 %s -fsyntax-only -verify -fblocks -Winvalid-noreturn

No need to pass in the -fblocks option.

> +__attribute__((noreturn)) void test1() {
> +  while(1){}
> +  return 0;

This function returns void, and so it should not return 0. In fact,
for this, you should be able to get away with just a declaration (and
not a definition).

> +}
> +
> +__attribute__((noreturn)) int test2() { // expected-warning{{function 'test1' declared 'noreturn' should not have a non-void return type}}
> +  while(1){}
> +  return 0;
> +}
> +
> +__attribute__((noreturn)) float test3() { // expected-warning{{function 'test1' declared 'noreturn' should not have a non-void return type}}
> +  while(1){}
> +  return 0.f;
> +}

I think a declaration only would be sufficient for these tests. Also,
it would be good to have a test for _Noreturn in C11 mode, and
[[noreturn]] in C++11 mode as well.

Otherwise, LGTM, but I would wait for secondary confirmation before committing.

~Aaron