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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to