[PATCH] Emit warning if define or undef reserved identifier or keyword.

[Reid Kleckner]

================
Comment at: include/clang/Basic/DiagnosticLexKinds.td:299
@@ +298,3 @@
+def warn_pp_defundef_reserved_ident : Warning<
+  "reserved identifier is used as macro name">, DefaultIgnore,
+  InGroup<ReservedIdAsMacro>;
----------------
Personally, I think it's confusing to have a warning group where some instances are on by default and others are off. We end up with a tri-state of default, on, off. Many users do things like `clang -w -Werror -Wthing1 -Wthing2 -Wthing3 ...` to explicitly use a curated set of warnings. It would be too bad if they couldn't use the on-by-default part of this warning due to lumping them into the same group.

Maybe we can split the group into -Wkeyword-macro and -Wreserved-id-macro? Does gcc have a flag name we should be using for compatibility?

================
Comment at: lib/Basic/IdentifierTable.cpp:216
@@ +215,3 @@
+static KeywordStatus GetKeywordStatus(const LangOptions &LangOpts,
+  tok::TokenKind K) {
+  switch (K) {
----------------
indentation is off

================
Comment at: lib/Lex/PPDirectives.cpp:111
@@ +110,3 @@
+
+static MacroDiag ShouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
+  StringRef Text = II->getName();
----------------
The LLVM naming conventions call for leading lower camel case function names, so this should be `shouldWarnOnMacroDef`.

================
Comment at: lib/Lex/PPDirectives.cpp:117
@@ +116,3 @@
+  if (Text.size() >= 2 && Text[0] == '_' &&
+    ((Text[1] >= 'A' && Text[1] <= 'Z') || Text[1] == '_'))
+    return MD_WarnStrict;
----------------
indentation

================
Comment at: lib/Lex/PPDirectives.cpp:117
@@ +116,3 @@
+  if (Text.size() >= 2 && Text[0] == '_' &&
+    ((Text[1] >= 'A' && Text[1] <= 'Z') || Text[1] == '_'))
+    return MD_WarnStrict;
----------------
rnk wrote:
> indentation
We have a table lookup based isUppercase() for this. Factoring this check out as a helper gives us a good place to put this quote from the C++ standard.

================
Comment at: lib/Lex/PPDirectives.cpp:133
@@ +132,3 @@
+
+static MacroDiag ShouldWarnOnMacroUndef(Preprocessor &PP, IdentifierInfo *II) {
+  if (II->isKeyword(PP.getLangOpts()))
----------------
s/Should/should/

================
Comment at: lib/Lex/PPDirectives.cpp:143
@@ +142,3 @@
+  if (Text.size() >= 2 && Text[0] == '_' &&
+    ((Text[1] >= 'A' && Text[1] <= 'Z') || Text[1] == '_'))
+    return MD_WarnStrict;
----------------
Reuse this code

================
Comment at: lib/Lex/PPDirectives.cpp:197
@@ +196,3 @@
+    PresumedLoc PLoc = getSourceManager().getPresumedLoc(MacroNameLoc, false);
+    WarnOnReserved = (strcmp(PLoc.getFilename(), "<built-in>") != 0);
+  }
----------------
There's got to be a better way to identify builtin macros...

http://reviews.llvm.org/D6194