Author: John Brawn Date: 2023-05-25T11:55:44+01:00 New Revision: 0123deb3a6f0a83095287f51b07c77b7b43ab847
URL: https://github.com/llvm/llvm-project/commit/0123deb3a6f0a83095287f51b07c77b7b43ab847 DIFF: https://github.com/llvm/llvm-project/commit/0123deb3a6f0a83095287f51b07c77b7b43ab847.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 0075f94e344ee..6ee73aa3c96fe 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -305,6 +305,8 @@ Improvements to Clang's diagnostics (`#58601 <https://github.com/llvm/llvm-project/issues/58601>`_) - Clang's `-Wshadow` warning now warns about shadowings by static local variables (`#62850: <https://github.com/llvm/llvm-project/issues/62850>`_). +- 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
