curdeius added inline comments.
================ Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:192 "}")); + EXPECT_EQ("#define M(x) x##x\n" + "namespace A M(x) {\n" ---------------- curdeius wrote: > zequanwu wrote: > > zequanwu wrote: > > > curdeius wrote: > > > > MyDeveloperDay wrote: > > > > > Can you test the A B case? We can’t have a space right? > > > > What's the rationale behind keeping `M(x)` in one case and not the > > > > other? > > > > How can you decide? > > > In the first one, we keep `M(x)` because we don't know that is `A` or > > > `M(x)` the name. > > > In the second one, because we see a `::`, so we know that's the name and > > > discard `M(x)`. > > Added a test with `A B`. > > In that case, we will have `// namespace A B`, since we don't know which > > one is the real name. Either one could be a macro that expands to an > > attribute. > In a different test case, could you add A to AttributeMacros and test that we > skip it like other attributes? Euh, actually, it's out of scope of this patch. Please ignore. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121269/new/ https://reviews.llvm.org/D121269 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits