[C] Diagnose use of C++ keywords in C (llvm#137234)

This adds a new diagnostic group, -Wc++-keyword, which is off by default
and grouped under -Wc++-compat. The diagnostic catches use of C++
keywords in C code.

This change additionally fixes an issue with -Wreserved-identifier not
diagnosing use of reserved identifiers in function parameter lists in a
function declaration which is not a definition.

Fixes llvm#21898

Author: AaronBallman

clang/docs/ReleaseNotes.rst

@@ -154,6 +154,8 @@ C Language Changes
 - Added ``-Wimplicit-void-ptr-cast``, grouped under ``-Wc++-compat``, which
   diagnoses implicit conversion from ``void *`` to another pointer type as
   being incompatible with C++. (#GH17792)
+- Added ``-Wc++-keyword``, grouped under ``-Wc++-compat``, which diagnoses when
+  a C++ keyword is used as an identifier in C. (#GH21898)
 - Added ``-Wc++-hidden-decl``, grouped under ``-Wc++-compat``, which diagnoses
   use of tag types which are visible in C but not visible in C++ due to scoping
   rules. e.g.,
@@ -482,6 +484,8 @@ Improvements to Clang's diagnostics
 - ``-Winitializer-overrides`` and ``-Wreorder-init-list`` are now grouped under
   the ``-Wc99-designator`` diagnostic group, as they also are about the
   behavior of the C99 feature as it was introduced into C++20. Fixes #GH47037
+- ``-Wreserved-identifier`` now fires on reserved parameter names in a function
+  declaration which is not a definition.
 
 Improvements to Clang's time-trace
 ----------------------------------

clang/include/clang/Basic/DiagnosticGroups.td

@@ -157,6 +157,7 @@ def C99Compat : DiagGroup<"c99-compat">;
 def C23Compat : DiagGroup<"c23-compat">;
 def : DiagGroup<"c2x-compat", [C23Compat]>;
 
+def CppKeywordInC : DiagGroup<"c++-keyword">;
 def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;
 def InitStringTooLongMissingNonString :
   DiagGroup<"unterminated-string-initialization">;
@@ -178,9 +179,9 @@ def ImplicitIntToEnumCast : DiagGroup<"implicit-int-enum-cast",
                                       [ImplicitEnumEnumCast]>;
 def TentativeDefnCompat : DiagGroup<"tentative-definition-compat">;
 def CXXCompat: DiagGroup<"c++-compat", [ImplicitVoidPtrCast, DefaultConstInit,
-                                        ImplicitIntToEnumCast, HiddenCppDecl,
-                                        InitStringTooLongForCpp,
-                                        TentativeDefnCompat,
+                                        ImplicitIntToEnumCast, CppKeywordInC,
+                                        HiddenCppDecl, TentativeDefnCompat,
+                                        InitStringTooLongForCpp,                                        
                                         DuplicateDeclSpecifier]>;
 
 def ExternCCompat : DiagGroup<"extern-c-compat">;

clang/include/clang/Basic/DiagnosticLexKinds.td

@@ -421,6 +421,9 @@ def warn_pp_macro_is_reserved_attribute_id : Warning<
 def warn_pp_objc_macro_redef_ignored : Warning<
   "ignoring redefinition of Objective-C qualifier macro">,
   InGroup<DiagGroup<"objc-macro-redefinition">>;
+def warn_pp_identifier_is_cpp_keyword : Warning<
+  "identifier %0 conflicts with a C++ keyword">,
+  InGroup<CppKeywordInC>, DefaultIgnore;
 
 def pp_invalid_string_literal : Warning<
   "invalid string literal, ignoring final '\\'">;

clang/include/clang/Basic/IdentifierTable.h

@@ -195,7 +195,11 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   LLVM_PREFERRED_TYPE(bool)
   unsigned IsFinal : 1;
 
-  // 22 bits left in a 64-bit word.
+  // True if this identifier would be a keyword in C++ mode.
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned IsKeywordInCpp : 1;
+
+  // 21 bits left in a 64-bit word.
 
   // Managed by the language front-end.
   void *FETokenInfo = nullptr;
@@ -212,7 +216,7 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
         IsFromAST(false), ChangedAfterLoad(false), FEChangedAfterLoad(false),
         RevertedTokenID(false), OutOfDate(false), IsModulesImport(false),
         IsMangledOpenMPVariantName(false), IsDeprecatedMacro(false),
-        IsRestrictExpansion(false), IsFinal(false) {}
+        IsRestrictExpansion(false), IsFinal(false), IsKeywordInCpp(false) {}
 
 public:
   IdentifierInfo(const IdentifierInfo &) = delete;
@@ -444,6 +448,10 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   }
   bool isCPlusPlusOperatorKeyword() const { return IsCPPOperatorKeyword; }
 
+  /// Return true if this identifier would be a keyword in C++ mode.
+  bool IsKeywordInCPlusPlus() const { return IsKeywordInCpp; }
+  void setIsKeywordInCPlusPlus(bool Val = true) { IsKeywordInCpp = Val; }
+
   /// Return true if this token is a keyword in the specified language.
   bool isKeyword(const LangOptions &LangOpts) const;
 
@@ -462,6 +470,7 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   /// If this returns false, we know that HandleIdentifier will not affect
   /// the token.
   bool isHandleIdentifierCase() const { return NeedsHandleIdentifier; }
+  void setHandleIdentifierCase(bool Val = true) { NeedsHandleIdentifier = Val; }
 
   /// Return true if the identifier in its current state was loaded
   /// from an AST file.

clang/include/clang/Basic/TokenKinds.def

@@ -1042,6 +1042,7 @@ ANNOTATION(embed)
 #undef TYPE_TRAIT_2
 #undef TYPE_TRAIT_1
 #undef TYPE_TRAIT
+#undef MODULES_KEYWORD
 #undef CXX20_KEYWORD
 #undef CXX11_KEYWORD
 #undef KEYWORD

clang/lib/Basic/IdentifierTable.cpp

@@ -250,6 +250,32 @@ static KeywordStatus getKeywordStatus(const LangOptions &LangOpts,
   return CurStatus;
 }
 
+static bool IsKeywordInCpp(unsigned Flags) {
+  while (Flags != 0) {
+    unsigned CurFlag = Flags & ~(Flags - 1);
+    Flags = Flags & ~CurFlag;
+    switch (static_cast<TokenKey>(CurFlag)) {
+    case KEYCXX:
+    case KEYCXX11:
+    case KEYCXX20:
+    case BOOLSUPPORT:
+    case WCHARSUPPORT:
+    case CHAR8SUPPORT:
+      return true;
+    default:
+      break; // Go to the next flag, try again.
+    }
+  }
+  return false;
+}
+
+static void MarkIdentifierAsKeywordInCpp(IdentifierTable &Table,
+                                         StringRef Name) {
+  IdentifierInfo &II = Table.get(Name, tok::identifier);
+  II.setIsKeywordInCPlusPlus();
+  II.setHandleIdentifierCase();
+}
+
 /// AddKeyword - This method is used to associate a token ID with specific
 /// identifiers because they are language keywords.  This causes the lexer to
 /// automatically map matching identifiers to specialized token codes.
@@ -258,8 +284,18 @@ static void AddKeyword(StringRef Keyword,
                        const LangOptions &LangOpts, IdentifierTable &Table) {
   KeywordStatus AddResult = getKeywordStatus(LangOpts, Flags);
 
-  // Don't add this keyword if disabled in this language.
-  if (AddResult == KS_Disabled) return;
+  // Don't add this keyword if disabled in this language and isn't otherwise
+  // special.
+  if (AddResult == KS_Disabled) {
+    // We do not consider any identifiers to be C++ keywords when in
+    // Objective-C because @ effectively introduces a custom grammar where C++
+    // keywords can be used (and similar for selectors). We could enable this
+    // for Objective-C, but it would require more logic to ensure we do not
+    // issue compatibility diagnostics in these cases.
+    if (!LangOpts.ObjC && IsKeywordInCpp(Flags))
+      MarkIdentifierAsKeywordInCpp(Table, Keyword);
+    return;
+  }
 
   IdentifierInfo &Info =
       Table.get(Keyword, AddResult == KS_Future ? tok::identifier : TokenCode);
@@ -304,9 +340,11 @@ void IdentifierTable::AddKeywords(const LangOptions &LangOpts) {
 #define ALIAS(NAME, TOK, FLAGS) \
   AddKeyword(StringRef(NAME), tok::kw_ ## TOK,  \
              FLAGS, LangOpts, *this);
-#define CXX_KEYWORD_OPERATOR(NAME, ALIAS) \
-  if (LangOpts.CXXOperatorNames)          \
-    AddCXXOperatorKeyword(StringRef(#NAME), tok::ALIAS, *this);
+#define CXX_KEYWORD_OPERATOR(NAME, ALIAS)                                      \
+  if (LangOpts.CXXOperatorNames)                                               \
+    AddCXXOperatorKeyword(StringRef(#NAME), tok::ALIAS, *this);                \
+  else                                                                         \
+    MarkIdentifierAsKeywordInCpp(*this, StringRef(#NAME));
 #define OBJC_AT_KEYWORD(NAME)  \
   if (LangOpts.ObjC)           \
     AddObjCKeyword(StringRef(#NAME), tok::objc_##NAME, *this);

clang/lib/Lex/Preprocessor.cpp

@@ -834,6 +834,11 @@ bool Preprocessor::HandleIdentifier(Token &Identifier) {
     II.setIsFutureCompatKeyword(false);
   }
 
+  // If this identifier would be a keyword in C++, diagnose as a compatibility
+  // issue.
+  if (II.IsKeywordInCPlusPlus() && !DisableMacroExpansion)
+    Diag(Identifier, diag::warn_pp_identifier_is_cpp_keyword) << &II;
+
   // If this is an extension token, diagnose its use.
   // We avoid diagnosing tokens that originate from macro definitions.
   // FIXME: This warning is disabled in cases where it shouldn't be,

clang/lib/Sema/SemaDecl.cpp

@@ -59,8 +59,10 @@
 #include "clang/Sema/SemaWasm.h"
 #include "clang/Sema/Template.h"
 #include "llvm/ADT/STLForwardCompat.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/TargetParser/Triple.h"
 #include <algorithm>
@@ -10424,6 +10426,15 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
   // Finally, we know we have the right number of parameters, install them.
   NewFD->setParams(Params);
 
+  // If this declarator is a declaration and not a definition, its parameters
+  // will not be pushed onto a scope chain. That means we will not issue any
+  // reserved identifier warnings for the declaration, but we will for the
+  // definition. Handle those here.
+  if (!D.isFunctionDefinition()) {
+    for (const ParmVarDecl *PVD : Params)
+      warnOnReservedIdentifier(PVD);
+  }
+
   if (D.getDeclSpec().isNoreturnSpecified())
     NewFD->addAttr(
         C11NoReturnAttr::Create(Context, D.getDeclSpec().getNoreturnSpecLoc()));

clang/test/OpenMP/assumes_messages.c

@@ -59,8 +59,14 @@
 #pragma omp begin assumes ext // expected-warning {{valid begin assumes clauses start with 'ext_', 'absent', 'contains', 'holds', 'no_openmp', 'no_openmp_routines', 'no_openmp_constructs', 'no_parallelism'; token will be ignored}}
 #pragma omp end assumes
 
-#pragma omp assumes ext_123(not allowed) // expected-warning {{'ext_123' clause should not be followed by arguments; tokens will be ignored}} expected-note {{the ignored tokens spans until here}}
-#pragma omp begin assumes ext_123(not allowed) // expected-warning {{'ext_123' clause should not be followed by arguments; tokens will be ignored}} expected-note {{the ignored tokens spans until here}}
+// FIXME: We should be getting an expected note about where the span of ignored
+// tokens ends. However, error recovery ends up lexing the 'not' token,
+// emitting a (silenced) diagnostic about use of a C++ keyword in C, and the
+// note gets associated with *that* (silenced) diagnostic. This is an existing
+// issue that also happens with error recovery of reserved identifiers or
+// extension tokens, but is unfortunate nonetheless.
+#pragma omp assumes ext_123(not allowed) // expected-warning {{'ext_123' clause should not be followed by arguments; tokens will be ignored}}
+#pragma omp begin assumes ext_123(not allowed) // expected-warning {{'ext_123' clause should not be followed by arguments; tokens will be ignored}}
 #pragma omp end assumes
 
 #pragma omp end assumes // expected-error {{'#pragma omp end assumes' with no matching '#pragma omp begin assumes'}}