john.brawn created this revision.
john.brawn added reviewers: aaron.ballman, nathanchance, mstorsjo.
Herald added a project: All.
john.brawn requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
D144654 <https://reviews.llvm.org/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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D151741
Files:
clang/lib/Lex/PPDirectives.cpp
clang/test/Preprocessor/macro-reserved.c
Index: clang/test/Preprocessor/macro-reserved.c
===================================================================
--- clang/test/Preprocessor/macro-reserved.c
+++ 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,7 @@
#undef _HAVE_X
#undef X__Y
#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
+#undef __INT32_TYPE__
// allowlisted definitions
#define while while
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -150,6 +150,30 @@
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();
@@ -3106,9 +3130,7 @@
// 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.
@@ -3189,11 +3211,8 @@
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())
Index: clang/test/Preprocessor/macro-reserved.c
===================================================================
--- clang/test/Preprocessor/macro-reserved.c
+++ 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,7 @@
#undef _HAVE_X
#undef X__Y
#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
+#undef __INT32_TYPE__
// allowlisted definitions
#define while while
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -150,6 +150,30 @@
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();
@@ -3106,9 +3130,7 @@
// 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.
@@ -3189,11 +3211,8 @@
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())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
