[clang] 22e3f58 - [Lex] Warn when defining or undefining any builtin macro
John Brawn via cfe-commits
cfe-commits at lists.llvm.org
Wed May 17 03:50:52 PDT 2023
Author: John Brawn
Date: 2023-05-17T11:49:49+01:00
New Revision: 22e3f587fd1ff97185014cb1ba723579ed2150d3
URL: https://github.com/llvm/llvm-project/commit/22e3f587fd1ff97185014cb1ba723579ed2150d3
DIFF: https://github.com/llvm/llvm-project/commit/22e3f587fd1ff97185014cb1ba723579ed2150d3.diff
LOG: [Lex] Warn when defining or undefining any builtin macro
Currently we warn when MI->isBuiltinMacro, but this is only true for
builtin macros that require processing when expanding. Checking
SourceMgr.isWrittenInBuiltinFile in addition to this will mean that
we catch all builtin macros, though we shouldn't warn on feature test
macros.
As part of doing this I've also moved the handling of undefining from
CheckMacroName to HandleUndefDirective, as it doesn't really make
sense to handle undefining in CheckMacroName but defining in
HandleDefineDirective. It would be nice to instead handle both in
CheckMacroName, but that isn't possible as the handling of defines
requires looking at what the name is being defined to.
Differential Revision: https://reviews.llvm.org/D144654
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Lex/PPDirectives.cpp
clang/test/Lexer/builtin_redef.c
clang/test/Preprocessor/macro-reserved.c
clang/test/Preprocessor/macro-reserved.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 77990f330e4bb..2f13ef96ba03f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -283,6 +283,8 @@ 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/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 62e51e133b3af..2066c61748efa 100644
--- 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,15 +319,6 @@ 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();
@@ -3012,6 +3003,12 @@ 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(
@@ -3083,15 +3080,9 @@ 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.
- 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() &&
+ if (getLangOpts().ObjC &&
+ SourceMgr.getFileID(OtherMI->getDefinitionLoc()) ==
+ getPredefinesFileID() &&
isObjCProtectedMacro(MacroNameTok.getIdentifierInfo())) {
// Warn if it changes the tokens.
if ((!getDiagnostics().getSuppressSystemWarnings() ||
@@ -3115,7 +3106,9 @@ 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())
+ if (OtherMI->isBuiltinMacro() ||
+ (SourceMgr.isWrittenInBuiltinFile(OtherMI->getDefinitionLoc()) &&
+ !isFeatureTestMacro(MacroNameTok.getIdentifierInfo()->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.
@@ -3195,6 +3188,14 @@ 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
index b0bb2a096be58..457e0b8af28e5 100644
--- a/clang/test/Lexer/builtin_redef.c
+++ b/clang/test/Lexer/builtin_redef.c
@@ -1,12 +1,24 @@
-// 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
+// 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
// 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
index 94245c024edec..14dbc9119943f 100644
--- a/clang/test/Preprocessor/macro-reserved.c
+++ b/clang/test/Preprocessor/macro-reserved.c
@@ -6,6 +6,7 @@
#define __cplusplus
#define _HAVE_X 0
#define X__Y
+#define __STDC__ 1 // expected-warning {{redefining builtin macro}}
#undef for
#undef final
@@ -13,6 +14,7 @@
#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
index c4f5ee91dd5a6..53bb3634bac4c 100644
--- 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
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
#define __cplusplus
// allowlisted definitions
More information about the cfe-commits
mailing list