Revert "[Lex] Warn when defining or undefining any builtin macro"

nico · nico · commit cf8c358dc941 · 2023-05-17T11:01:33.000-04:00
This reverts commit 22e3f58. Breaks check-clang on arm, see https://reviews.llvm.org/D144654#4349954 Also reverts follow-up "[AArch64] Don't redefine _LP64 and __LP64__" This reverts commit e55d52c.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -285,8 +285,6 @@ Improvements to Clang's diagnostics
 - Clang constexpr evaluator now prints subobject's name instead of its type in notes
   when a constexpr variable has uninitialized subobjects after its constructor call.
   (`#58601 <https://github.com/llvm/llvm-project/issues/58601>`_)
-- Clang now warns when any predefined macro is undefined or redefined, instead
-  of only some of them.
 
 Bug Fixes in This Version
 -------------------------
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
@@ -340,6 +340,12 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
       getTriple().isOSBinFormatELF())
     Builder.defineMacro("__ELF__");
 
+  // Target properties.
+  if (!getTriple().isOSWindows() && getTriple().isArch64Bit()) {
+    Builder.defineMacro("_LP64");
+    Builder.defineMacro("__LP64__");
+  }
+
   std::string CodeModel = getTargetOpts().CodeModel;
   if (CodeModel == "default")
     CodeModel = "small";
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
@@ -109,52 +109,52 @@ enum PPElifDiag {
   PED_Elifndef
 };
 
-static bool isFeatureTestMacro(StringRef MacroName) {
-  // list from:
-  // * https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
-  // * https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
-  // * man 7 feature_test_macros
-  // The list must be sorted for correct binary search.
-  static constexpr StringRef ReservedMacro[] = {
-      "_ATFILE_SOURCE",
-      "_BSD_SOURCE",
-      "_CRT_NONSTDC_NO_WARNINGS",
-      "_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
-      "_CRT_SECURE_NO_WARNINGS",
-      "_FILE_OFFSET_BITS",
-      "_FORTIFY_SOURCE",
-      "_GLIBCXX_ASSERTIONS",
-      "_GLIBCXX_CONCEPT_CHECKS",
-      "_GLIBCXX_DEBUG",
-      "_GLIBCXX_DEBUG_PEDANTIC",
-      "_GLIBCXX_PARALLEL",
-      "_GLIBCXX_PARALLEL_ASSERTIONS",
-      "_GLIBCXX_SANITIZE_VECTOR",
-      "_GLIBCXX_USE_CXX11_ABI",
-      "_GLIBCXX_USE_DEPRECATED",
-      "_GNU_SOURCE",
-      "_ISOC11_SOURCE",
-      "_ISOC95_SOURCE",
-      "_ISOC99_SOURCE",
-      "_LARGEFILE64_SOURCE",
-      "_POSIX_C_SOURCE",
-      "_REENTRANT",
-      "_SVID_SOURCE",
-      "_THREAD_SAFE",
-      "_XOPEN_SOURCE",
-      "_XOPEN_SOURCE_EXTENDED",
-      "__STDCPP_WANT_MATH_SPEC_FUNCS__",
-      "__STDC_FORMAT_MACROS",
-  };
-  return std::binary_search(std::begin(ReservedMacro), std::end(ReservedMacro),
-                            MacroName);
-}
-
 static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
   const LangOptions &Lang = PP.getLangOpts();
+  if (isReservedInAllContexts(II->isReserved(Lang))) {
+    // list from:
+    // - https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+    // - https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
+    // - man 7 feature_test_macros
+    // The list must be sorted for correct binary search.
+    static constexpr StringRef ReservedMacro[] = {
+        "_ATFILE_SOURCE",
+        "_BSD_SOURCE",
+        "_CRT_NONSTDC_NO_WARNINGS",
+        "_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
+        "_CRT_SECURE_NO_WARNINGS",
+        "_FILE_OFFSET_BITS",
+        "_FORTIFY_SOURCE",
+        "_GLIBCXX_ASSERTIONS",
+        "_GLIBCXX_CONCEPT_CHECKS",
+        "_GLIBCXX_DEBUG",
+        "_GLIBCXX_DEBUG_PEDANTIC",
+        "_GLIBCXX_PARALLEL",
+        "_GLIBCXX_PARALLEL_ASSERTIONS",
+        "_GLIBCXX_SANITIZE_VECTOR",
+        "_GLIBCXX_USE_CXX11_ABI",
+        "_GLIBCXX_USE_DEPRECATED",
+        "_GNU_SOURCE",
+        "_ISOC11_SOURCE",
+        "_ISOC95_SOURCE",
+        "_ISOC99_SOURCE",
+        "_LARGEFILE64_SOURCE",
+        "_POSIX_C_SOURCE",
+        "_REENTRANT",
+        "_SVID_SOURCE",
+        "_THREAD_SAFE",
+        "_XOPEN_SOURCE",
+        "_XOPEN_SOURCE_EXTENDED",
+        "__STDCPP_WANT_MATH_SPEC_FUNCS__",
+        "__STDC_FORMAT_MACROS",
+    };
+    if (std::binary_search(std::begin(ReservedMacro), std::end(ReservedMacro),
+                           II->getName()))
+      return MD_NoWarn;
+
+    return MD_ReservedMacro;
+  }
   StringRef Text = II->getName();
-  if (isReservedInAllContexts(II->isReserved(Lang)))
-    return isFeatureTestMacro(Text) ? MD_NoWarn : MD_ReservedMacro;
   if (II->isKeyword(Lang))
     return MD_KeywordDef;
   if (Lang.CPlusPlus11 && (Text.equals("override") || Text.equals("final")))
@@ -319,6 +319,15 @@ bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef,
     return Diag(MacroNameTok, diag::err_defined_macro_name);
   }
 
+  if (isDefineUndef == MU_Undef) {
+    auto *MI = getMacroInfo(II);
+    if (MI && MI->isBuiltinMacro()) {
+      // Warn if undefining "__LINE__" and other builtins, per C99 6.10.8/4
+      // and C++ [cpp.predefined]p4], but allow it as an extension.
+      Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);
+    }
+  }
+
   // If defining/undefining reserved identifier or a keyword, we need to issue
   // a warning.
   SourceLocation MacroNameLoc = MacroNameTok.getLocation();
@@ -3003,12 +3012,6 @@ MacroInfo *Preprocessor::ReadOptionalMacroParameterListAndBody(
   MI->setTokens(Tokens, BP);
   return MI;
 }
-
-static bool isObjCProtectedMacro(const IdentifierInfo *II) {
-  return II->isStr("__strong") || II->isStr("__weak") ||
-         II->isStr("__unsafe_unretained") || II->isStr("__autoreleasing");
-}
-
 /// HandleDefineDirective - Implements \#define.  This consumes the entire macro
 /// line then lets the caller lex the next real token.
 void Preprocessor::HandleDefineDirective(
@@ -3080,9 +3083,15 @@ void Preprocessor::HandleDefineDirective(
     // In Objective-C, ignore attempts to directly redefine the builtin
     // definitions of the ownership qualifiers.  It's still possible to
     // #undef them.
-    if (getLangOpts().ObjC &&
-        SourceMgr.getFileID(OtherMI->getDefinitionLoc()) ==
-            getPredefinesFileID() &&
+    auto isObjCProtectedMacro = [](const IdentifierInfo *II) -> bool {
+      return II->isStr("__strong") ||
+             II->isStr("__weak") ||
+             II->isStr("__unsafe_unretained") ||
+             II->isStr("__autoreleasing");
+    };
+   if (getLangOpts().ObjC &&
+        SourceMgr.getFileID(OtherMI->getDefinitionLoc())
+          == getPredefinesFileID() &&
         isObjCProtectedMacro(MacroNameTok.getIdentifierInfo())) {
       // Warn if it changes the tokens.
       if ((!getDiagnostics().getSuppressSystemWarnings() ||
@@ -3106,9 +3115,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 (OtherMI->isBuiltinMacro())
         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.
@@ -3188,14 +3195,6 @@ void Preprocessor::HandleUndefDirective() {
     if (!MI->isUsed() && MI->isWarnIfUnused())
       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)))
-      Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);
-
     if (MI->isWarnIfUnused())
       WarnUnusedMacroLocs.erase(MI->getDefinitionLoc());
 
diff --git a/clang/test/Lexer/builtin_redef.c b/clang/test/Lexer/builtin_redef.c
@@ -1,24 +1,12 @@
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
-// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
+// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
 
 // CHECK-WARN: <command line>:{{.*}} warning: redefining builtin macro
-// CHECK-WARN-NEXT: #define __TIME__ 1234
 // CHECK-WARN: <command line>:{{.*}} warning: undefining builtin macro
-// CHECK-WARN-NEXT: #undef __DATE__
-// CHECK-WARN: <command line>:{{.*}} warning: redefining builtin macro
-// CHECK-WARN-NEXT: #define __STDC__ 1
-// CHECK-WARN: <command line>:{{.*}} warning: undefining builtin macro
-// CHECK-WARN-NEXT: #undef __STDC_HOSTED__
 
 // CHECK-ERR: <command line>:{{.*}} error: redefining builtin macro
-// CHECK-ERR-NEXT: #define __TIME__ 1234
-// CHECK-ERR: <command line>:{{.*}} error: undefining builtin macro
-// CHECK-ERR-NEXT: #undef __DATE__
-// CHECK-ERR: <command line>:{{.*}} error: redefining builtin macro
-// CHECK-ERR-NEXT: #define __STDC__ 1
 // CHECK-ERR: <command line>:{{.*}} error: undefining builtin macro
-// CHECK-ERR-NEXT: #undef __STDC_HOSTED__
 
 int n = __TIME__;
 __DATE__
diff --git a/clang/test/Preprocessor/macro-reserved.c b/clang/test/Preprocessor/macro-reserved.c
@@ -6,15 +6,13 @@
 #define __cplusplus
 #define _HAVE_X 0
 #define X__Y
-#define __STDC__ 1 // expected-warning {{redefining builtin macro}}
 
 #undef for
 #undef final
 #undef __HAVE_X
 #undef __cplusplus
 #undef _HAVE_X
 #undef X__Y
-#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
 
 // allowlisted definitions
 #define while while
diff --git a/clang/test/Preprocessor/macro-reserved.cpp b/clang/test/Preprocessor/macro-reserved.cpp
@@ -12,7 +12,7 @@
 #undef _HAVE_X
 #undef X__Y
 
-#undef __cplusplus // expected-warning {{undefining builtin macro}}
+#undef __cplusplus
 #define __cplusplus
 
 // allowlisted definitions
