fix(cxx_indexer): emit ref to class from ctor (#4400)

* fix(cxx_indexer): emit ref to class from ctor

* fix(cxx_indexer): don't add TypeSourceInfo for implicit constructors

* chore: remove debug logging statement

kythe/cxx/indexer/cxx/IndexerASTHooks.cc

@@ -1077,6 +1077,20 @@ void IndexerASTVisitor::VisitRecordDeclComment(
   }
 }
 
+bool IndexerASTVisitor::TraverseCXXConstructorDecl(
+    clang::CXXConstructorDecl* CD) {
+  auto DNI = CD->getNameInfo();
+  if (DNI.getNamedTypeInfo() == nullptr && !CD->isImplicit()) {
+    // Clang does not currently set the NamedTypeInfo for constructors,
+    // but does for destructors and conversion operators.  As such, work
+    // around this here. Implicit constructors do not get TypeSourceInfo.
+    DNI.setNamedTypeInfo(Context.getTrivialTypeSourceInfo(
+        QualType(CD->getParent()->getTypeForDecl(), 0), CD->getLocation()));
+  }
+  return RecursiveASTVisitor::TraverseCXXConstructorDecl(CD) &&
+         TraverseDeclarationNameInfo(DNI);
+}
+
 bool IndexerASTVisitor::TraverseDecl(clang::Decl* Decl) {
   if (ShouldStopIndexing()) {
     return false;
@@ -1952,11 +1966,23 @@ NodeSet IndexerASTVisitor::RecordTypeLocSpellingLocation(clang::TypeLoc TL) {
 
 bool IndexerASTVisitor::TraverseDeclarationNameInfo(
     clang::DeclarationNameInfo NameInfo) {
-  // For ConversionFunctions this leads to duplicate edges as the return value
-  // is visited both here and via TraverseFunctionProtoTypeLoc.
-  if (NameInfo.getName().getNameKind() ==
-      clang::DeclarationName::CXXConversionFunctionName) {
-    return true;
+  switch (NameInfo.getName().getNameKind()) {
+    case DeclarationName::CXXConversionFunctionName:
+      // For ConversionFunctions this leads to duplicate edges as the return
+      // value is visited both here and via TraverseFunctionProtoTypeLoc.
+      return true;
+    case DeclarationName::CXXConstructorName:
+      // The default visitation uses the null TypeSourceInfo, which we work
+      // around in TraverseCXXConstructorDecl by re-traversing with a
+      // manually constructed TSI.  In order to avoid duplicate edges, suppress
+      // the default visitation.
+      // Note: if this is resolved in Clang, tests will fail due to duplicate
+      // edges and this workaround can be removed.
+      if (NameInfo.getNamedTypeInfo() == nullptr) {
+        return true;
+      }
+    default:
+      break;
   }
   return Base::TraverseDeclarationNameInfo(NameInfo);
 }

kythe/cxx/indexer/cxx/IndexerASTHooks.h

@@ -173,6 +173,7 @@ class IndexerASTVisitor : public clang::RecursiveASTVisitor<IndexerASTVisitor> {
   bool VisitEnumConstantDecl(const clang::EnumConstantDecl* Decl);
   bool VisitFunctionDecl(clang::FunctionDecl* Decl);
   bool TraverseDecl(clang::Decl* Decl);
+  bool TraverseCXXConstructorDecl(clang::CXXConstructorDecl* CD);
 
   bool TraverseConstructorInitializer(clang::CXXCtorInitializer* Init);
   bool TraverseCXXNewExpr(clang::CXXNewExpr* E);

kythe/cxx/indexer/cxx/testdata/BUILD

@@ -1805,6 +1805,12 @@ cc_indexer_test(
     tags = ["rec"],
 )
 
+cc_indexer_test(
+    name = "rec_ctor_dtor_ref_class",
+    srcs = ["rec/rec_ctor_dtor_ref_class.cc"],
+    tags = ["rec"],
+)
+
 test_suite(
     name = "indexer_rec",
     tags = ["rec"],

kythe/cxx/indexer/cxx/testdata/rec/rec_ctor_dtor_ref_class.cc

@@ -0,0 +1,15 @@
+// Tests that class constructor and destructor reference the class.
+
+//- @C defines/binding CStruct
+struct C {
+  //- @C ref CStruct
+  C();
+  //- @C ref CStruct
+  explicit C(int);
+  //- @C ref CStruct
+  ~C();
+};
+
+//- @#0C ref CStruct
+//- @#1C ref CStruct
+C::C() {}