Improve diagnostics if _Noreturn is placed after a function declarator. (This sometimes happens when a macro is used that expands to either the GNU noreturn attribute or _Noreturn.)

llvm-svn: 221630

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -121,6 +121,8 @@ def ext_c11_alignment : Extension<
 
 def ext_c11_noreturn : Extension<
   "_Noreturn functions are a C11-specific feature">, InGroup<C11>;
+def err_c11_noreturn_misplaced : Error<
+  "'_Noreturn' keyword must precede function declarator">;
 
 def ext_gnu_indirect_goto : Extension<
   "use of GNU indirect-goto extension">, InGroup<GNULabelsAsValue>;

clang/lib/Parse/ParseDecl.cpp

@@ -1601,9 +1601,30 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
   // appropriate function scope after the function Decl has been constructed.
   // These will be parsed in ParseFunctionDefinition or ParseLexedAttrList.
   LateParsedAttrList LateParsedAttrs(true);
-  if (D.isFunctionDeclarator())
+  if (D.isFunctionDeclarator()) {
     MaybeParseGNUAttributes(D, &LateParsedAttrs);
 
+    // The _Noreturn keyword can't appear here, unlike the GNU noreturn
+    // attribute. If we find the keyword here, tell the user to put it
+    // at the start instead.
+    if (Tok.is(tok::kw__Noreturn)) {
+      SourceLocation Loc = ConsumeToken();
+      const char *PrevSpec;
+      unsigned DiagID;
+
+      // We can offer a fixit if it's valid to mark this function as _Noreturn
+      // and we don't have any other declarators in this declaration.
+      bool Fixit = !DS.setFunctionSpecNoreturn(Loc, PrevSpec, DiagID);
+      MaybeParseGNUAttributes(D, &LateParsedAttrs);
+      Fixit &= Tok.is(tok::semi) || Tok.is(tok::l_brace) || Tok.is(tok::kw_try);
+
+      Diag(Loc, diag::err_c11_noreturn_misplaced)
+          << (Fixit ? FixItHint::CreateRemoval(Loc) : FixItHint())
+          << (Fixit ? FixItHint::CreateInsertion(D.getLocStart(), "_Noreturn ")
+                    : FixItHint());
+    }
+  }
+
   // Check to see if we have a function *definition* which must have a body.
   if (D.isFunctionDeclarator() &&
       // Look at the next token to make sure that this isn't a function

clang/test/FixIt/fixit-errors.c

@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
 // RUN: cp %s %t
 // RUN: not %clang_cc1 -pedantic -fixit -x c %t
-// RUN: %clang_cc1 -pedantic -Werror -x c %t
+// RUN: %clang_cc1 -pedantic -Werror -Wno-invalid-noreturn -x c %t
 
 /* This is a test of the various code modification hints that are
    provided as part of warning or extension diagnostics. All of the
@@ -21,3 +21,11 @@ struct Point *get_origin();
 void test_point() {
   (void)get_origin->x; // expected-error {{base of member reference is a function; perhaps you meant to call it with no arguments?}}
 }
+
+void noreturn_1() _Noreturn; // expected-error {{must precede function declarator}}
+void noreturn_1() {
+  return; // expected-warning {{should not return}}
+}
+void noreturn_2() _Noreturn { // expected-error {{must precede function declarator}}
+  return; // expected-warning {{should not return}}
+}

clang/test/Parser/c11-noreturn.c

@@ -4,7 +4,7 @@
 _Noreturn int f();
 int _Noreturn f(); // expected-note {{previous}}
 int f _Noreturn(); // expected-error {{expected ';'}} expected-error 2{{}}
-int f() _Noreturn; // expected-error {{expected ';'}} expected-warning {{does not declare anything}} expected-error {{'_Noreturn' can only appear on functions}}
+int f() _Noreturn; // expected-error {{'_Noreturn' keyword must precede function declarator}}
 
 _Noreturn char c1; // expected-error {{'_Noreturn' can only appear on functions}}
 char _Noreturn c2; // expected-error {{'_Noreturn' can only appear on functions}}