[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

cseo added a comment. In D144654#4381893 , @nickdesaulniers wrote: > In D144654#4380488 , @john.brawn > wrote: > >> gcc has the same warning so I wasn't expecting this cause to change >> problems, but looking

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

nickdesaulniers added a comment. In D144654#4380488 , @john.brawn wrote: > gcc has the same warning so I wasn't expecting this cause to change problems, > but looking more closely at gcc's behaviour it looks like it only warns for > some builtin

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

mstorsjo added a comment. FWIW, I've also run into noisy warnings caused by this, in a few places. D151662 fixes a bunch of clang's tests when run in MinGW configurations, and

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn added a comment. gcc has the same warning so I wasn't expecting this cause to change problems, but looking more closely at gcc's behaviour it looks like it only warns for some builtin macros and not others (though glancing over the gcc source code it's not clear which macros and for

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

nathanchance added subscribers: nickdesaulniers, nathanchance. nathanchance added a comment. For what it's worth, this change is quite noisy for the Linux kernel, as there are certain macros undefined in header files that are included in many other headers. Additionally, at least one new

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn added a comment. D150966 should fix the failure that was seen in buildbots, so I've now re-committed this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144654/new/ https://reviews.llvm.org/D144654

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

thakis added a comment. Reverted in cf8c358dc9414e3de9f65eebdfdcb66dd5817857 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144654/new/ https://reviews.llvm.org/D144654 ___ cfe-commits mailing

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

thakis added a comment. In D144654#4349908 , @john.brawn wrote: > This patch caused a failure in AArch64 buildbots due to > AArch64TargetInfo::getTargetDefines redefining _LP64 and __LP64__. Fixed in >

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn added a comment. This patch caused a failure in AArch64 buildbots due to AArch64TargetInfo::getTargetDefines redefining _LP64 and __LP64__. Fixed in https://reviews.llvm.org/rGe55d52cd34fb7a6a6617639d147b9d0abaceeeab. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG22e3f587fd1f: [Lex] Warn when defining or undefining any builtin macro (authored by john.brawn). Changed prior to commit:

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, though precommit CI wasn't able to run on this, so please pay attention to the bots when you land in case there's fallout. Comment at:

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn added inline comments. Comment at: clang/test/Preprocessor/macro-reserved.cpp:15 -#undef __cplusplus +#undef __cplusplus // expected-warning {{undefining builtin macro}} #define __cplusplus aaron.ballman wrote: > Why do we diagnose the undef but

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn updated this revision to Diff 522683. john.brawn added a comment. Adjusted in order to make clang-format happy. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144654/new/ https://reviews.llvm.org/D144654 Files: clang/docs/ReleaseNotes.rst clang/lib/Lex/PPDirectives.cpp

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

aaron.ballman added inline comments. Comment at: clang/test/Preprocessor/macro-reserved.cpp:15 -#undef __cplusplus +#undef __cplusplus // expected-warning {{undefining builtin macro}} #define __cplusplus Why do we diagnose the undef but not the define?

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn updated this revision to Diff 522215. john.brawn edited the summary of this revision. john.brawn added a comment. I've gone with handling the _GNU_SOURCE problem in the clang-tidy tests by re-using the list of feature test macros in shouldWarnOnMacroDef, and not warning if there's a

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn planned changes to this revision. john.brawn added a comment. The clang-tidy failures in pre-merge are because - clang-tidy tests use the compile database from building llvm (though you have to manually copy it otherwise the tests silently pass without actually running the test,

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn updated this revision to Diff 521263. john.brawn added a comment. Rebasing now that the patches this depends on have been committed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144654/new/ https://reviews.llvm.org/D144654 Files: clang/docs/ReleaseNotes.rst

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

aaron.ballman added a comment. It looks like there's still relevant precommit CI failures: `clang-tidy/checkers/readability/identifier-naming-case-violation.cpp` looked related to this patch. Comment at: clang/lib/Lex/PPDirectives.cpp:3189-3192 +if ((MI->isBuiltinMacro()

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:3189-3192 +if ((MI->isBuiltinMacro() || + SourceMgr.isWrittenInBuiltinFile(MI->getDefinitionLoc())) && +!(getLangOpts().ObjC && isObjCProtectedMacro(II))) + Diag(MacroNameTok,

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

aaron.ballman added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:3189-3192 +if ((MI->isBuiltinMacro() || + SourceMgr.isWrittenInBuiltinFile(MI->getDefinitionLoc())) && +!(getLangOpts().ObjC && isObjCProtectedMacro(II))) +

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn added a comment. D145691 should fix the libc++ CI failures. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144654/new/ https://reviews.llvm.org/D144654 ___ cfe-commits mailing list

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn updated this revision to Diff 503305. john.brawn added a comment. Add command line test and release note, and run clang-format. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144654/new/ https://reviews.llvm.org/D144654 Files: clang/docs/ReleaseNotes.rst

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

john.brawn added a comment. In D144654#4156430 , @aaron.ballman wrote: > This seems to cause precommit CI failures that should be addressed, and it > should also have a release note about the fix. Failures in CI should be fixed by D145397

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

aaron.ballman added a comment. This seems to cause precommit CI failures that should be addressed, and it should also have a release note about the fix. Also, should this cover things like `clang -U__cplusplus -D__STDC__=1 foo.cpp` as well? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

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