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

Reply via email to