[PATCH] NoReturn Warning: Non-Void Return Type

[Aaron Ballman]

> 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>;

Let's avoid the double-negative. How about:

function %0 declared 'noreturn' should have a 'void' return type

>  def err_noreturn_block_has_return_expr : Error<
>    "block declared 'noreturn' should not return">;
>  def err_noreturn_missing_on_first_decl : Error<
> 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;
>    return NewFD;
>  }
>
> 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,12 @@
> +// RUN: %clang_cc1 %s -fsyntax-only -verify -Winvalid-noreturn

Might want to pass -std=c11 to be safe.

> +__attribute__((noreturn)) void test1();
> +
> +__attribute__((noreturn)) int test2(); // expected-warning{{function 'test2' declared 'noreturn' should not have a non-void return type}}
> +
> +__attribute__((noreturn)) float test3(); // expected-warning{{function 'test3' declared 'noreturn' should not have a non-void return type}}
> +
> +_Noreturn void test4();
> +
> +_Noreturn int test5(); // expected-warning{{function 'test5' declared 'noreturn' should not have a non-void return type}}
> +
> +_Noreturn float test6(); // expected-warning{{function 'test6' declared 'noreturn' should not have a non-void return type}}
> Index: test/SemaCXX/warn-noreturn-return-type.cpp
> ===================================================================
> --- test/SemaCXX/warn-noreturn-return-type.cpp
> +++ test/SemaCXX/warn-noreturn-return-type.cpp
> @@ -0,0 +1,6 @@
> +// RUN: %clang_cc1 %s -fsyntax-only -verify -Winvalid-noreturn -std=c++11
> +void test1 [[noreturn]]();
> +
> +int test2 [[noreturn]](); // expected-warning{{function 'test2' declared 'noreturn' should not have a non-void return type}}
> +
> +float test3 [[noreturn]](); // expected-warning{{function 'test3' declared 'noreturn' should not have a non-void return type}}

The [[noreturn]] should precede the declaration, not the parameter
list. I believe it's technically legal where it's at, but the
convention is for it to precede.

Otherwise, patch LGTM! I would wait for secondary confirmation before
committing though.

Thanks!

~Aaron

On Fri, Jan 3, 2014 at 4:16 PM, Michael Bao <mike.h.bao at gmail.com> wrote:
>   Oh, finally figured it out. Sorry for the horrendous amounts of spam.
>
>   Made modifications according to Aaron's suggestions.
>
>   1) Pass in NewFD instead of NewFD->getDeclName() to the Diagnostic.
>   2) Removed -fblocks compilation flag from test.
>   3) Made the tests only use declarations.
>   4) Fixed a grammatical error in the warning.
>   5) Added in _Noreturn and [[noreturn]] tests for C11 and C++11.
>
> http://llvm-reviews.chandlerc.com/D2507
>
> CHANGE SINCE LAST DIFF
>   http://llvm-reviews.chandlerc.com/D2507?vs=6342&id=6345#toc
>
> Files:
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/Sema/SemaDecl.cpp
>   test/Sema/warn-noreturn-return-type.c
>   test/SemaCXX/warn-noreturn-return-type.cpp
>
> 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: 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;
>    return NewFD;
>  }
>
> 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,12 @@
> +// RUN: %clang_cc1 %s -fsyntax-only -verify -Winvalid-noreturn
> +__attribute__((noreturn)) void test1();
> +
> +__attribute__((noreturn)) int test2(); // expected-warning{{function 'test2' declared 'noreturn' should not have a non-void return type}}
> +
> +__attribute__((noreturn)) float test3(); // expected-warning{{function 'test3' declared 'noreturn' should not have a non-void return type}}
> +
> +_Noreturn void test4();
> +
> +_Noreturn int test5(); // expected-warning{{function 'test5' declared 'noreturn' should not have a non-void return type}}
> +
> +_Noreturn float test6(); // expected-warning{{function 'test6' declared 'noreturn' should not have a non-void return type}}
> Index: test/SemaCXX/warn-noreturn-return-type.cpp
> ===================================================================
> --- test/SemaCXX/warn-noreturn-return-type.cpp
> +++ test/SemaCXX/warn-noreturn-return-type.cpp
> @@ -0,0 +1,6 @@
> +// RUN: %clang_cc1 %s -fsyntax-only -verify -Winvalid-noreturn -std=c++11
> +void test1 [[noreturn]]();
> +
> +int test2 [[noreturn]](); // expected-warning{{function 'test2' declared 'noreturn' should not have a non-void return type}}
> +
> +float test3 [[noreturn]](); // expected-warning{{function 'test3' declared 'noreturn' should not have a non-void return type}}
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>