[PATCH] D120931: [clang-format] Fix namespace format when the name is a macro expansion
Zequan Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 4 12:48:24 PST 2022
zequanwu added inline comments.
================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:112
+ EXPECT_EQ("#define M(x) x##x\n"
+ "namespace [[deprecated(\"foo\")]] A::inline M(x)::A {\n"
+ "int i;\n"
----------------
MyDeveloperDay wrote:
> Is this 2 bugs in one? I notice you also handling attributes? is this a different bug? (if so it should really be separate (but we can let it slide as long as the tests are thorough)
>
> can you test:
>
> ```
> namespace /* comment */ [[ xxx ]] /* comment */ A {
> int i;
> int j;
> } // namespace A
>
> namespace /* comment */ [[ xxx ]] A {
> int i;
> int j;
> } // namespace A
>
> namespace /* comment */ [[ xxx ]] /* comment */ M(x) {
> int i;
> int j;
> } // namespace M(x)
>
> namespace /* comment */ [[ xxx ]] /* comment */ A::M(x) {
> int i;
> int j;
> } // namespace A::M(x)
>
> namespace /* comment */ [[ xxx ]] /* comment */ M(x) /* comment */ {
> int i;
> int j;
> } // namespace M(x)
>
> namespace /* comment */ [[ xxx ]] /* comment */ A::M(x) /* comment */ {
> int i;
> int j;
> } // namespace A::M(x)
> ```
> Is this 2 bugs in one? I notice you also handling attributes?
No. This tests with attribute is here to test that candidate name doesn't include attributes, but that is unnecessary. Added the 6 tests above for testing that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120931/new/
https://reviews.llvm.org/D120931
More information about the cfe-commits
mailing list