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

[Erich Keane via cfe-commits]

================
@@ -6107,6 +6109,29 @@ static bool isFromSystemHeader(SourceManager &SM, const Decl *D) {
          SM.isInSystemMacro(D->getLocation());
 }
 
+static bool isKeywordInCPlusPlus(const Sema &S, const IdentifierInfo *II) {
+  if (!II)
+    return false;
+
+  // Build a static map of identifiers for all of the keywords in C++ that are
+  // not keywords in C. This allows us to do pointer comparisons instead of
+  // string comparisons when deciding whether the given identifier is a keyword
+  // or not. Note, this treats all keywords as being enabled, regardless of the
+  // setting of other language options. It intentionally disables the modules
+  // keywords because those are conditional keywords, so may be safe to use.
+  static llvm::SmallPtrSet<IdentifierInfo *, 32> Keywords;
+  if (Keywords.empty()) {
+#define MODULES_KEYWORD(NAME)
+#define KEYWORD(NAME, FLAGS)                                                   \
+  Keywords.insert(&S.getPreprocessor().getIdentifierTable().get(#NAME));
+#define CXX_KEYWORD_OPERATOR(NAME, TOK)                                        \
+  Keywords.insert(&S.getPreprocessor().getIdentifierTable().get(#NAME));
+#include "clang/Basic/TokenKinds.def"
+  }
+
+  return Keywords.contains(II);
----------------
erichkeane wrote:

This function is interesting.  I like the approach.

The `insert` here is obviously pretty slow, but all of those are only happening 1x per TU and a 'small' number (which, btw, how accurate is the 32 here? we NEED to make sure, perhaps via assert, that we always fall in the 'small' here).

An alternative implementation idea here...

In IdentifierTable, 'start' the TU by creating all the CXXKeywords in 1 place, via 1 allocation.  Then add them to the IdentifierTable.  That then makes this lookup:

`return II >= IdentTable.FirstCXXKeyword && II <= IdentTable.LastCXXKeyword`

The result is this is effectively 'free' to check, at a slight startup time/memory cost(basically, time to do the ABOVE getIdentifierTable.get calls).

That said, the 'cost' of this warning, now that you have `isIgnored` checked, is pretty minor I presume? So perhaps not worth the effort.

WDYT?

https://github.com/llvm/llvm-project/pull/137234