Warn on duplicate function specifier

This patch fixes PR8264. Duplicate qualifiers already are diagnozed,
now the same diagnostics is issued for duplicate function specifiers.

Differential Revision: http://llvm-reviews.chandlerc.com/D2025

llvm-svn: 194559

7 files changed, 121 insertions, 23 deletions

diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index 773e31edbdb..9e1511d63a2 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -337,6 +337,7 @@ private:
 
   // function-specifier
   unsigned FS_inline_specified : 1;
+  unsigned FS_forceinline_specified: 1;
   unsigned FS_virtual_specified : 1;
   unsigned FS_explicit_specified : 1;
   unsigned FS_noreturn_specified : 1;
@@ -381,6 +382,7 @@ private:
   SourceRange TypeofParensRange;
   SourceLocation TQ_constLoc, TQ_restrictLoc, TQ_volatileLoc, TQ_atomicLoc;
   SourceLocation FS_inlineLoc, FS_virtualLoc, FS_explicitLoc, FS_noreturnLoc;
+  SourceLocation FS_forceinlineLoc;
   SourceLocation FriendLoc, ModulePrivateLoc, ConstexprLoc;
 
   WrittenBuiltinSpecs writtenBS;
@@ -419,6 +421,7 @@ public:
       TypeSpecOwned(false),
       TypeQualifiers(TQ_unspecified),
       FS_inline_specified(false),
+      FS_forceinline_specified(false),
       FS_virtual_specified(false),
       FS_explicit_specified(false),
       FS_noreturn_specified(false),
@@ -532,8 +535,12 @@ public:
   }
 
   // function-specifier
-  bool isInlineSpecified() const { return FS_inline_specified; }
-  SourceLocation getInlineSpecLoc() const { return FS_inlineLoc; }
+  bool isInlineSpecified() const {
+    return FS_inline_specified | FS_forceinline_specified;
+  }
+  SourceLocation getInlineSpecLoc() const {
+    return FS_inline_specified ? FS_inlineLoc : FS_forceinlineLoc;
+  }
 
   bool isVirtualSpecified() const { return FS_virtual_specified; }
   SourceLocation getVirtualSpecLoc() const { return FS_virtualLoc; }
@@ -547,6 +554,8 @@ public:
   void ClearFunctionSpecs() {
     FS_inline_specified = false;
     FS_inlineLoc = SourceLocation();
+    FS_forceinline_specified = false;
+    FS_forceinlineLoc = SourceLocation();
     FS_virtual_specified = false;
     FS_virtualLoc = SourceLocation();
     FS_explicit_specified = false;
@@ -634,10 +643,16 @@ public:
   bool SetTypeQual(TQ T, SourceLocation Loc, const char *&PrevSpec,
                    unsigned &DiagID, const LangOptions &Lang);
 
-  bool setFunctionSpecInline(SourceLocation Loc);
-  bool setFunctionSpecVirtual(SourceLocation Loc);
-  bool setFunctionSpecExplicit(SourceLocation Loc);
-  bool setFunctionSpecNoreturn(SourceLocation Loc);
+  bool setFunctionSpecInline(SourceLocation Loc, const char *&PrevSpec,
+                             unsigned &DiagID);
+  bool setFunctionSpecForceInline(SourceLocation Loc, const char *&PrevSpec,
+                                  unsigned &DiagID);
+  bool setFunctionSpecVirtual(SourceLocation Loc, const char *&PrevSpec,
+                              unsigned &DiagID);
+  bool setFunctionSpecExplicit(SourceLocation Loc, const char *&PrevSpec,
+                               unsigned &DiagID);
+  bool setFunctionSpecNoreturn(SourceLocation Loc, const char *&PrevSpec,
+                               unsigned &DiagID);
 
   bool SetFriendSpec(SourceLocation Loc, const char *&PrevSpec,
                      unsigned &DiagID);
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index c5904933133..36d704d31a9 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2730,7 +2730,7 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
 
     // Microsoft single token adornments.
     case tok::kw___forceinline: {
-      isInvalid = DS.setFunctionSpecInline(Loc);
+      isInvalid = DS.setFunctionSpecForceInline(Loc, PrevSpec, DiagID);
       IdentifierInfo *AttrName = Tok.getIdentifierInfo();
       SourceLocation AttrNameLoc = Tok.getLocation();
       // FIXME: This does not work correctly if it is set to be a declspec
@@ -2822,18 +2822,18 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
 
     // function-specifier
     case tok::kw_inline:
-      isInvalid = DS.setFunctionSpecInline(Loc);
+      isInvalid = DS.setFunctionSpecInline(Loc, PrevSpec, DiagID);
       break;
     case tok::kw_virtual:
-      isInvalid = DS.setFunctionSpecVirtual(Loc);
+      isInvalid = DS.setFunctionSpecVirtual(Loc, PrevSpec, DiagID);
       break;
     case tok::kw_explicit:
-      isInvalid = DS.setFunctionSpecExplicit(Loc);
+      isInvalid = DS.setFunctionSpecExplicit(Loc, PrevSpec, DiagID);
       break;
     case tok::kw__Noreturn:
       if (!getLangOpts().C11)
         Diag(Loc, diag::ext_c11_noreturn);
-      isInvalid = DS.setFunctionSpecNoreturn(Loc);
+      isInvalid = DS.setFunctionSpecNoreturn(Loc, PrevSpec, DiagID);
       break;
 
     // alignment-specifier
diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index 46914f68040..538c16eeb25 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -349,7 +349,7 @@ unsigned DeclSpec::getParsedSpecifiers() const {
     Res |= PQ_TypeSpecifier;
 
   if (FS_inline_specified || FS_virtual_specified || FS_explicit_specified ||
-      FS_noreturn_specified)
+      FS_noreturn_specified || FS_forceinline_specified)
     Res |= PQ_FunctionSpecifier;
   return Res;
 }
@@ -739,9 +739,10 @@ bool DeclSpec::SetTypeSpecError() {
 
 bool DeclSpec::SetTypeQual(TQ T, SourceLocation Loc, const char *&PrevSpec,
                            unsigned &DiagID, const LangOptions &Lang) {
-  // Duplicates are permitted in C99, but are not permitted in C++. However,
-  // since this is likely not what the user intended, we will always warn.  We
-  // do not need to set the qualifier's location since we already have it.
+  // Duplicates are permitted in C99 onwards, but are not permitted in C89 or
+  // C++.  However, since this is likely not what the user intended, we will
+  // always warn.  We do not need to set the qualifier's location since we
+  // already have it.
   if (TypeQualifiers & T) {
     bool IsExtension = true;
     if (Lang.C99)
@@ -761,29 +762,72 @@ bool DeclSpec::SetTypeQual(TQ T, SourceLocation Loc, const char *&PrevSpec,
   llvm_unreachable("Unknown type qualifier!");
 }
 
-bool DeclSpec::setFunctionSpecInline(SourceLocation Loc) {
-  // 'inline inline' is ok.
+bool DeclSpec::setFunctionSpecInline(SourceLocation Loc, const char *&PrevSpec,
+                                     unsigned &DiagID) {
+  // 'inline inline' is ok.  However, since this is likely not what the user
+  // intended, we will always warn, similar to duplicates of type qualifiers.
+  if (FS_inline_specified) {
+    DiagID = diag::warn_duplicate_declspec;
+    PrevSpec = "inline";
+    return true;
+  }
   FS_inline_specified = true;
   FS_inlineLoc = Loc;
   return false;
 }
 
-bool DeclSpec::setFunctionSpecVirtual(SourceLocation Loc) {
-  // 'virtual virtual' is ok.
+bool DeclSpec::setFunctionSpecForceInline(SourceLocation Loc, const char *&PrevSpec,
+                                          unsigned &DiagID) {
+  if (FS_forceinline_specified) {
+    DiagID = diag::warn_duplicate_declspec;
+    PrevSpec = "__forceinline";
+    return true;
+  }
+  FS_forceinline_specified = true;
+  FS_forceinlineLoc = Loc;
+  return false;
+}
+
+bool DeclSpec::setFunctionSpecVirtual(SourceLocation Loc,
+                                      const char *&PrevSpec,
+                                      unsigned &DiagID) {
+  // 'virtual virtual' is ok, but warn as this is likely not what the user
+  // intended.
+  if (FS_virtual_specified) {
+    DiagID = diag::warn_duplicate_declspec;
+    PrevSpec = "virtual";
+    return true;
+  }
   FS_virtual_specified = true;
   FS_virtualLoc = Loc;
   return false;
 }
 
-bool DeclSpec::setFunctionSpecExplicit(SourceLocation Loc) {
-  // 'explicit explicit' is ok.
+bool DeclSpec::setFunctionSpecExplicit(SourceLocation Loc,
+                                       const char *&PrevSpec,
+                                       unsigned &DiagID) {
+  // 'explicit explicit' is ok, but warn as this is likely not what the user
+  // intended.
+  if (FS_explicit_specified) {
+    DiagID = diag::warn_duplicate_declspec;
+    PrevSpec = "explicit";
+    return true;
+  }
   FS_explicit_specified = true;
   FS_explicitLoc = Loc;
   return false;
 }
 
-bool DeclSpec::setFunctionSpecNoreturn(SourceLocation Loc) {
-  // '_Noreturn _Noreturn' is ok.
+bool DeclSpec::setFunctionSpecNoreturn(SourceLocation Loc,
+                                       const char *&PrevSpec,
+                                       unsigned &DiagID) {
+  // '_Noreturn _Noreturn' is ok, but warn as this is likely not what the user
+  // intended.
+  if (FS_noreturn_specified) {
+    DiagID = diag::warn_duplicate_declspec;
+    PrevSpec = "_Noreturn";
+    return true;
+  }
   FS_noreturn_specified = true;
   FS_noreturnLoc = Loc;
   return false;
diff --git a/clang/test/Parser/MicrosoftExtensions.c b/clang/test/Parser/MicrosoftExtensions.c
index de933f9e5d6..5e1139338b8 100644
--- a/clang/test/Parser/MicrosoftExtensions.c
+++ b/clang/test/Parser/MicrosoftExtensions.c
@@ -30,6 +30,20 @@ void __forceinline InterlockedBitTestAndSet (long *Base, long Bit)
   };
 #endif
 }
+
+// Both inline and __forceinline is OK.
+inline void __forceinline pr8264() {
+}
+__forceinline void inline pr8264_1() {
+}
+void inline __forceinline pr8264_2() {
+}
+void __forceinline inline pr8264_3() {
+}
+// But duplicate __forceinline causes warning.
+void __forceinline __forceinline pr8264_4() {  // expected-warning{{duplicate '__forceinline' declaration specifier}}
+}
+
 _inline int foo99() { return 99; }
 
 void test_ms_alignof_alias() {
diff --git a/clang/test/Sema/declspec.c b/clang/test/Sema/declspec.c
index 30c009201ce..d810632eeea 100644
--- a/clang/test/Sema/declspec.c
+++ b/clang/test/Sema/declspec.c
@@ -36,3 +36,16 @@ void test2() {}
 struct test3s {
 } // expected-error {{expected ';' after struct}}
 typedef int test3g;
+
+// PR8264
+const const int pr8264_1 = 0;  // expected-warning {{duplicate 'const' declaration specifier}}
+volatile volatile int pr8264_2;  // expected-warning {{duplicate 'volatile' declaration specifier}}
+char * restrict restrict pr8264_3;  // expected-warning {{duplicate 'restrict' declaration specifier}}
+
+extern extern int pr8264_4;  // expected-warning {{duplicate 'extern' declaration specifier}}
+void pr8264_5() {
+  register register int x;  // expected-warning {{duplicate 'register' declaration specifier}}
+}
+
+inline inline void pr8264_6() {}  // expected-warning {{duplicate 'inline' declaration specifier}}
+_Noreturn _Noreturn void pr8264_7();  // expected-warning {{duplicate '_Noreturn' declaration specifier}}
diff --git a/clang/test/SemaCXX/explicit.cpp b/clang/test/SemaCXX/explicit.cpp
index e47f132fc6b..1c4d7704511 100644
--- a/clang/test/SemaCXX/explicit.cpp
+++ b/clang/test/SemaCXX/explicit.cpp
@@ -240,3 +240,9 @@ namespace Conversion {
     nfp(1); // expected-error {{type 'NotFP' does not provide a call operator}}
   }
 }
+
+namespace pr8264 {
+  struct Test {
+  explicit explicit Test(int x);  // expected-warning{{duplicate 'explicit' declaration specifier}}
+  };
+}
diff --git a/clang/test/SemaCXX/virtuals.cpp b/clang/test/SemaCXX/virtuals.cpp
index a340e9d86b6..6b8231d4e1e 100644
--- a/clang/test/SemaCXX/virtuals.cpp
+++ b/clang/test/SemaCXX/virtuals.cpp
@@ -45,3 +45,9 @@ namespace rdar9670557 {
     func *h = 0;
   };
 }
+
+namespace pr8264 {
+  struct Test {
+    virtual virtual void func();  // expected-warning {{duplicate 'virtual' declaration specifier}}
+  };
+}