[clang] 844e953 - [Lex] Only warn on defining or undefining language-defined builtins

[John Brawn via cfe-commits]

Author: John Brawn
Date: 2023-06-01T17:37:50+01:00
New Revision: 844e9534c6d99ddb6bada740839760fa24d17cb6

URL: https://github.com/llvm/llvm-project/commit/844e9534c6d99ddb6bada740839760fa24d17cb6
DIFF: https://github.com/llvm/llvm-project/commit/844e9534c6d99ddb6bada740839760fa24d17cb6.diff

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

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.

Differential Revision: https://reviews.llvm.org/D151741

Added: 
    clang/test/Preprocessor/undef-x86.c

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index e83da5c573871..f133a50dd2ab6 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -150,6 +150,30 @@ static bool isFeatureTestMacro(StringRef MacroName) {
                             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();
@@ -3107,9 +3131,7 @@ void Preprocessor::HandleDefineDirective(
 
       // 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.
@@ -3190,11 +3212,8 @@ void Preprocessor::HandleUndefDirective() {
       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())

diff  --git a/clang/test/Preprocessor/macro-reserved.c b/clang/test/Preprocessor/macro-reserved.c
index 14dbc9119943f..6026a9f60730e 100644
--- a/clang/test/Preprocessor/macro-reserved.c
+++ b/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,12 @@
 #undef _HAVE_X
 #undef X__Y
 #undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
+#undef __INT32_TYPE__
+#undef __UINT32_TYPE__
+#undef __UINTPTR_TYPE__
+#undef __UINT64_TYPE__
+#undef __INT64_TYPE__
+#undef __OPTIMIZE__
 
 // allowlisted definitions
 #define while while

diff  --git a/clang/test/Preprocessor/undef-x86.c b/clang/test/Preprocessor/undef-x86.c
new file mode 100644
index 0000000000000..91f16d3aae3ab
--- /dev/null
+++ b/clang/test/Preprocessor/undef-x86.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple=i386-none-none -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple=x86_64-none-none -fsyntax-only -verify %s
+
+// Check that we can undefine triple-specific defines without warning
+// expected-no-diagnostics
+#undef __i386
+#undef __i386__
+#undef i386
+#undef __amd64
+#undef __amd64__
+#undef __x86_64
+#undef __x86_64__