[PATCH] D150966: [clang] Don't define predefined macros multiple times

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG78bf8a0a2212: [clang] Dont define predefined macros multiple times (authored by john.brawn). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D150966: [clang] Don't define predefined macros multiple times

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/Preprocessor/predefined-macros-no-warnings.c:5 +// warnings suppressed by default. +// RUN: %clang_cc1 %s -E -o /dev/null

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn updated this revision to Diff 525165. john.brawn added a comment. Use -Eonly in test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150966/new/ https://reviews.llvm.org/D150966 Files: clang/lib/Basic/Targets/AArch64.cpp clang/lib/Basic/Targets/ARM.cpp

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:241-242 Builder.defineMacro("__ARM_FEATURE_QRDMX", "1"); - Builder.defineMacro("__ARM_FEATURE_ATOMICS", "1"); - Builder.defineMacro("__ARM_FEATURE_CRC32", "1"); }

[PATCH] D150966: [clang] Don't define predefined macros multiple times

aaron.ballman added a comment. Basically LG though I had a question about the test case. Comment at: clang/lib/Basic/Targets/AArch64.cpp:241-242 Builder.defineMacro("__ARM_FEATURE_QRDMX", "1"); - Builder.defineMacro("__ARM_FEATURE_ATOMICS", "1"); -

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn marked an inline comment as done. john.brawn added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:241-242 Builder.defineMacro("__ARM_FEATURE_QRDMX", "1"); - Builder.defineMacro("__ARM_FEATURE_ATOMICS", "1"); -

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn updated this revision to Diff 524686. john.brawn added a comment. Add VE test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150966/new/ https://reviews.llvm.org/D150966 Files: clang/lib/Basic/Targets/AArch64.cpp clang/lib/Basic/Targets/ARM.cpp

[PATCH] D150966: [clang] Don't define predefined macros multiple times

aaron.ballman added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:241-242 Builder.defineMacro("__ARM_FEATURE_QRDMX", "1"); - Builder.defineMacro("__ARM_FEATURE_ATOMICS", "1"); - Builder.defineMacro("__ARM_FEATURE_CRC32", "1"); }

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:241-242 Builder.defineMacro("__ARM_FEATURE_QRDMX", "1"); - Builder.defineMacro("__ARM_FEATURE_ATOMICS", "1"); - Builder.defineMacro("__ARM_FEATURE_CRC32", "1"); }

[PATCH] D150966: [clang] Don't define predefined macros multiple times

aaron.ballman added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:241-242 Builder.defineMacro("__ARM_FEATURE_QRDMX", "1"); - Builder.defineMacro("__ARM_FEATURE_ATOMICS", "1"); - Builder.defineMacro("__ARM_FEATURE_CRC32", "1"); } Hmm, is

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn added a comment. In D150966#4356779 , @barannikov88 wrote: > Check that the predefined macros don't contain anything that causes a > warning > > What is 'anything', exactly? Does it include duplicate predefined macros, or > it tests

[PATCH] D150966: [clang] Don't define predefined macros multiple times

barannikov88 added a comment. In D150966#4356695 , @john.brawn wrote: >> Q: What is it the -Wsystem-header can warn about? I mean, no system header >> is included by the test, what can go wrong? > > The buffer is treated as a system header. I've

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn added a comment. In D150966#4356403 , @barannikov88 wrote: > Is it possible/worth to add an assertion to Builder.defineMacro to enforce > this one definition rule? It just appends a string to a raw_ostream, so it's not possible to check

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn updated this revision to Diff 523803. john.brawn marked 2 inline comments as done. john.brawn added a comment. Send test output to /dev/null CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150966/new/ https://reviews.llvm.org/D150966 Files:

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn updated this revision to Diff 523785. john.brawn added a comment. Remove redundant braces, explain more in test comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150966/new/ https://reviews.llvm.org/D150966 Files: clang/lib/Basic/Targets/AArch64.cpp

[PATCH] D150966: [clang] Don't define predefined macros multiple times

barannikov88 added a comment. Q: What is it the -Wsystem-header can warn about? I mean, no system header is included by the test, what can go wrong? Comment at: clang/test/Preprocessor/predefined-macros-no-warnings.c:2 +// Check that the predefined macros don't contain

[PATCH] D150966: [clang] Don't define predefined macros multiple times

barannikov88 added a comment. Is it possible/worth to add an assertion to Builder.defineMacro to enforce this one definition rule? Comment at: clang/lib/Frontend/InitPreprocessor.cpp:1305 +Builder.defineMacro("__ELF__"); + } + Redundant braces.

[PATCH] D150966: [clang] Don't define predefined macros multiple times

john.brawn created this revision. john.brawn added reviewers: aaron.ballman, thakis, arichardson, pratlucas. Herald added subscribers: luke, abrachet, frasercrmck, phosek, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01,