[PATCH] D151741: [Lex] Only warn on defining or undefining language-defined builtins

[John Brawn via Phabricator via cfe-commits]

john.brawn created this revision.
john.brawn added reviewers: aaron.ballman, nathanchance, mstorsjo.
Herald added a project: All.
john.brawn requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D144654 <https://reviews.llvm.org/D144654> made it so that we warn on any defining or undefining of builtin macros. However the C and C++ standards only forbid the defining or undefining of macros defined in the language standard itself, but clang defines more macros than those and warning on those may not be helpful.

Resolve this by only warning if the builtin macro name is the name of a macro defined by the language. This is done in a way that removes some of the existing checks, as those were made redundant by restricting the warning in this way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151741

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/macro-reserved.c


Index: clang/test/Preprocessor/macro-reserved.c
===================================================================
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -7,6 +7,7 @@
 #define _HAVE_X 0
 #define X__Y
 #define __STDC__ 1 // expected-warning {{redefining builtin macro}}
+#define __clang__ 1
 
 #undef for
 #undef final
@@ -15,6 +16,7 @@
 #undef _HAVE_X
 #undef X__Y
 #undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
+#undef __INT32_TYPE__
 
 // allowlisted definitions
 #define while while
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -150,6 +150,30 @@
                             MacroName);
 }
 
+static bool isLanguageDefinedBuiltin(const SourceManager &SourceMgr,
+                                     const MacroInfo *MI,
+                                     const StringRef MacroName) {
+  // If this is a macro with special handling (like __LINE__) then it's language
+  // defined.
+  if (MI->isBuiltinMacro())
+    return true;
+  // Builtin macros are defined in the builtin file
+  if (!SourceMgr.isWrittenInBuiltinFile(MI->getDefinitionLoc()))
+    return false;
+  // C defines macros starting with __STDC, and C++ defines macros starting with
+  // __STDCPP
+  if (MacroName.startswith("__STDC"))
+    return true;
+  // C++ defines the __cplusplus macro
+  if (MacroName == "__cplusplus")
+    return true;
+  // C++ defines various feature-test macros starting with __cpp
+  if (MacroName.startswith("__cpp"))
+    return true;
+  // Anything else isn't language-defined
+  return false;
+}
+
 static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
   const LangOptions &Lang = PP.getLangOpts();
   StringRef Text = II->getName();
@@ -3106,9 +3130,7 @@
 
       // Warn if defining "__LINE__" and other builtins, per C99 6.10.8/4 and
       // C++ [cpp.predefined]p4, but allow it as an extension.
-      if (OtherMI->isBuiltinMacro() ||
-          (SourceMgr.isWrittenInBuiltinFile(OtherMI->getDefinitionLoc()) &&
-           !isFeatureTestMacro(MacroNameTok.getIdentifierInfo()->getName())))
+      if (isLanguageDefinedBuiltin(SourceMgr, OtherMI, II->getName()))
         Diag(MacroNameTok, diag::ext_pp_redef_builtin_macro);
       // Macros must be identical.  This means all tokens and whitespace
       // separation must be the same.  C99 6.10.3p2.
@@ -3189,11 +3211,8 @@
       Diag(MI->getDefinitionLoc(), diag::pp_macro_not_used);
 
     // Warn if undefining "__LINE__" and other builtins, per C99 6.10.8/4 and
-    // C++ [cpp.predefined]p4, but allow it as an extension. Don't warn if this
-    // is an Objective-C builtin macro though.
-    if ((MI->isBuiltinMacro() ||
-         SourceMgr.isWrittenInBuiltinFile(MI->getDefinitionLoc())) &&
-        !(getLangOpts().ObjC && isObjCProtectedMacro(II)))
+    // C++ [cpp.predefined]p4, but allow it as an extension.
+    if (isLanguageDefinedBuiltin(SourceMgr, MI, II->getName()))
       Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);
 
     if (MI->isWarnIfUnused())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D151741.526731.patch
Type: text/x-patch
Size: 3190 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230530/f559b2f9/attachment.bin>