john.brawn created this revision.
john.brawn added reviewers: rsmith, jansvoboda11, rjmccall.
Herald added a project: All.
john.brawn requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D144654
Files:
clang/lib/Lex/PPDirectives.cpp
clang/test/Preprocessor/macro-reserved.c
clang/test/Preprocessor/macro-reserved.cpp
Index: clang/test/Preprocessor/macro-reserved.cpp
===================================================================
--- clang/test/Preprocessor/macro-reserved.cpp
+++ 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
Index: clang/test/Preprocessor/macro-reserved.c
===================================================================
--- clang/test/Preprocessor/macro-reserved.c
+++ 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
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -319,15 +319,6 @@
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();
@@ -3004,6 +2995,14 @@
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(
@@ -3075,13 +3074,7 @@
// 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 &&
+ if (getLangOpts().ObjC &&
SourceMgr.getFileID(OtherMI->getDefinitionLoc())
== getPredefinesFileID() &&
isObjCProtectedMacro(MacroNameTok.getIdentifierInfo())) {
@@ -3107,7 +3100,8 @@
// 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()))
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.
@@ -3187,6 +3181,14 @@
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());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
