[clangd] Handle the missing constructor initializers in findExplicitReferences.

[ilya-biryukov]

LGTM, thanks!

clang-tools-extra/clangd/FindTarget.cpp

646

This looks misindented. Could you clang-format?

[ilya-biryukov]

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

844

Could you also add a test with the initializer containing a base class? It produces a different kind of CXXConstructorInitializer

[merge_guards_bot]

Build result: fail - 33623 tests passed, 1 failed and 462 were skipped.

failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt, ninja_check_all-log.txt, CMakeCache.txt

[ilya-biryukov]

NIT: there's a typo in the revision title: should be constructor, not consturctor

[hokein]

• fix some issues in the existing implementation;
• add tests for base/delegating initializer;

[hokein]

In D69241#1716787, @ilya-biryukov wrote:

NIT: there's a typo in the revision title: should be constructor, not consturctor

Done.

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

844

Done.

While adding more tests, I discover some issues in the implementation, also fixed them in this patch, please review the patch again.

[merge_guards_bot]

Build result: fail - 59521 tests passed, 1 failed and 805 were skipped.

failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt, ninja_check_all-log.txt, CMakeCache.txt

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

683

I believe base initializers are the only ones left, but the comment suggests there are more cases. Maybe change to:

// Base initializers are handled by visting the type loc.

Or am I missing something?

[hokein]

clang-tools-extra/clangd/FindTarget.cpp

683

there are others, delegating initializers.

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

683

Ah, interesting... Could we add them to the tests too?

[hokein]

clang-tools-extra/clangd/FindTarget.cpp

683

it has been added in the tests already, without the change here, the test will trigger the assert(CCI->isAnyMemberInitializer());.

// delegating initializer
class $10^Foo {
  $11^Foo(int$12^);
  $13^Foo(): $14^Foo(111) {}
};

[merge_guards_bot]

Build result: fail - 59521 tests passed, 1 failed and 805 were skipped.

failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt, ninja_check_all-log.txt, CMakeCache.txt

clang-tools-extra/clangd/FindTarget.cpp

if (!L.getNestedNameSpecifier())
       return true;
     visitNode(DynTypedNode::create(L));
     // Inner type is missing information about its qualifier, skip it.
     if (auto TL = L.getTypeLoc())
       TypeLocsToSkip.insert(TL.getBeginLoc().getRawEncoding());
     return RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(L);
   }
 
+  bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
+   visitNode(DynTypedNode::create(*Init));
+    return RecursiveASTVisitor::TraverseConstructorInitializer(Init);
+  }
+
 private:
   /// Obtain information about a reference directly defined in \p N. Does not
   /// recurse into child nodes, e.g. do not expect references for constructor
   /// initializers
   ///
   /// Any of the fields in the returned structure can be empty, but not all of
   /// them, e.g.
   ///   - for implicitly generated nodes (e.g. MemberExpr from range-based-for),
if (const TypeLoc *TL = N.get<TypeLoc>())
       return refInTypeLoc(*TL);
     if (const CXXCtorInitializer *CCI = N.get<CXXCtorInitializer>()) {
       if (CCI->isBaseInitializer())
         return refInTypeLoc(CCI->getBaseClassLoc());
       assert(CCI->isAnyMemberInitializer());
       return {ReferenceLoc{NestedNameSpecifierLoc(),
                            CCI->getMemberLocation(),
                            /*IsDecl=*/false,
                            {CCI->getAnyMember()}}};
     }
     // We do not have location information for other nodes (QualType, etc)
     return {};
   }
 
   void visitNode(DynTypedNode N) {
     for (const auto &R : explicitReference(N))
       reportReference(R, N);

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
               $0^wrapper<$1^func> $2^w;
               $3^FuncParam();
             }
         )cpp",
            "0: targets = {wrapper<&func>}\n"
            "1: targets = {func}\n"
            "2: targets = {w}, decl\n"
            "3: targets = {FuncParam}\n"},
+          // declaration references.
           {R"cpp(
              namespace ns {}
              class S {};
              void foo() {
                class $0^Foo { $1^Foo(); ~$2^Foo(); int $3^field; };
                int $4^Var;
                enum $5^E { $6^ABC };
                typedef int $7^INT;
                using $8^INT2 = int;
                namespace $9^NS = $10^ns;
              }
            )cpp",
            "0: targets = {Foo}, decl\n"
            "1: targets = {foo()::Foo::Foo}, decl\n"
            "2: targets = {Foo}\n"
            "3: targets = {foo()::Foo::field}, decl\n"
            "4: targets = {Var}, decl\n"
            "5: targets = {E}, decl\n"
            "6: targets = {foo()::ABC}, decl\n"
            "7: targets = {INT}, decl\n"
            "8: targets = {INT2}, decl\n"
            "9: targets = {NS}, decl\n"
            "10: targets = {ns}\n"},
+          // cxx constructor initializer.
+          {R"cpp(
+            void foo() {
+              class $0^X {
+                int $1^abc;
+                $2^X(): $3^abc() {}
+              };
+            }
+           )cpp",
+           "0: targets = {X}, decl\n"
+           "1: targets = {foo()::X::abc}, decl\n"
+           "2: targets = {foo()::X::X}, decl\n"
+           "3: targets = {foo()::X::abc}\n"}
       };
 
   for (const auto &C : Cases) {
     llvm::StringRef ExpectedCode = C.first;
     llvm::StringRef ExpectedRefs = C.second;
 
     auto Actual =
         annotateReferencesInFoo(llvm::Annotations(ExpectedCode).code());
     EXPECT_EQ(ExpectedCode, Actual.AnnotatedCode);
     EXPECT_EQ(ExpectedRefs, Actual.DumpedReferences) << ExpectedCode;
   }
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang