[PATCH] D120931: [clang-format] Fix namespace format when the name is a macro expansion

[Zequan Wu via Phabricator via cfe-commits]

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:
> zequanwu wrote:
> > 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.
> Where are you checking the addition of this
> 
> ```
> if (Tok && Tok->is(tok::l_square)) {
>       int NestLevel = 1;
>       while (Tok && NestLevel > 0) {
>         Tok = Tok->getNextNonComment();
>         if (Tok) {
>           if (Tok->is(tok::l_square))
>             ++NestLevel;
>           if (Tok->is(tok::r_square))
>             --NestLevel;
>         }
>       }
>       if (Tok)
>         Tok = Tok->getNextNonComment();
>     }
> ```
That part of code skips attribute so that `FirstNSName` doesn't add attribute string into its name.
In the following test case. it's tested. 
```
namespace /* comment  */ [[ xxx ]]   /* comment */  A::M(x) {
int i;
int j;
}  // namespace  A::M(x)
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120931/new/

https://reviews.llvm.org/D120931