[clang] Do not consider the template arguments of bases to be bases themselves

[nridge]

Summary

Fixes https://github.com/clangd/clangd/issues/504

[nridge]

Fix a typo

[hokein]

clang/lib/Index/IndexTypeSourceInfo.cpp

168

nit: this could be simplified by calling the base::TraverseTemplateSpecializationTypeLoc

clang/unittests/Index/IndexTests.cpp

337

nit: I think we need a test in clang, clang/test/Index/Core/index-source.cpp.

[nridge]

clang/lib/Index/IndexTypeSourceInfo.cpp

168

I don't see how, given that WalkUpFromTemplateSpecializationTypeLoc()needs to be called with Relations containing RelationIsBaseOf, while TraverseTemplateArgumentLoc() needs to be called with Relations empty.

[hokein]

clang/lib/Index/IndexTypeSourceInfo.cpp

168

you're right. I misthought the above 4 lines are exactly the same as the one in RAV.

175

if something bad happens (TraverseTemplateArgumentLoc returns false), we will not restore the Relations, I think we need a RAII pattern (llvm::make_scope_exit) to restore the Relations.

[nridge]

Address review comments

[nridge]

format

clang/lib/Index/IndexTypeSourceInfo.cpp

 //===- IndexTypeSourceInfo.cpp - Indexing types ---------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "IndexingContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "llvm/ADT/ScopeExit.h"
 
 using namespace clang;
 using namespace index;
 
 namespace {
 
 class TypeIndexer : public RecursiveASTVisitor<TypeIndexer> {
   IndexingContext &IndexCtx;
bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
     if (!T)
       return true;
     HandleTemplateSpecializationTypeLoc(
         T->getTemplateName(), TL.getTemplateNameLoc(), T->getAsCXXRecordDecl(),
         T->isTypeAlias());
     return true;
   }
 
+  bool TraverseTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
+    if (!WalkUpFromTemplateSpecializationTypeLoc(TL))
+      return false;
+    if (!TraverseTemplateName(TL.getTypePtr()->getTemplateName()))
+      return false;
+
+    // The relations we have to `Parent` do not apply to our template arguments,
+    // so clear them while visiting the args.
+    SmallVector<SymbolRelation, 3> SavedRelations = Relations;
+    Relations.clear();
+    auto ResetSavedRelations =
+        llvm::make_scope_exit([&] { this->Relations = SavedRelations; });
+    for (unsigned I = 0, E = TL.getNumArgs(); I != E; ++I) {
+      if (!TraverseTemplateArgumentLoc(TL.getArgLoc(I)))
+        return false;
+    }
+
+    return true;
+  }
+
   bool VisitDeducedTemplateSpecializationTypeLoc(DeducedTemplateSpecializationTypeLoc TL) {
     auto *T = TL.getTypePtr();
     if (!T)
       return true;
     HandleTemplateSpecializationTypeLoc(
         T->getTemplateName(), TL.getTemplateNameLoc(), T->getAsCXXRecordDecl(),
         /*IsTypeAlias=*/false);
     return true;

clang/test/Index/Core/index-source.cpp

class SubclassOffsetof : public Struct {
   void foo() {
     OFFSET_OF_(SubclassOffsetof, field);
 // CHECK: [[@LINE-1]]:34 | field/C | field | c:@N@index_offsetof@S@Struct@FI@field | <no-cgname> | Ref,RelCont | rel: 1
 // CHECK-NEXT: RelCont | foo | c:@N@index_offsetof@S@SubclassOffsetof@F@foo#
   }
 };
 
 }
+
+namespace clangd_issue_504 {
+class A {};
+template <typename> class B {};
+class C : B<A> {};
+// CHECK: [[@LINE-1]]:13 | class/C++ | A | c:@N@clangd_issue_504@S@A | <no-cgname> | Ref,RelCont | rel: 1
+// CHECK-NEXT: RelCont | C | c:@N@clangd_issue_504@S@C
+} // namespace clangd_issue_504

clang/unittests/Index/IndexTests.cpp

TEST(IndexTest, VisitDefaultArgs) {
   Opts.IndexFunctionLocals = true;
   Opts.IndexParametersInDeclarations = true;
   tooling::runToolOnCode(std::make_unique<IndexAction>(Index, Opts), Code);
   EXPECT_THAT(Index->Symbols,
               Contains(AllOf(QName("var"), HasRole(SymbolRole::Reference),
                              WrittenAt(Position(3, 20)))));
 }
 
+TEST(IndexTest, RelationBaseOf) {
+  std::string Code = R"cpp(
+    class A {};
+    template <typename> class B {};
+    class C : B<A> {};
+  )cpp";
+  auto Index = std::make_shared<Indexer>();
+  tooling::runToolOnCode(std::make_unique<IndexAction>(Index), Code);
+  // A should not be the base of anything.
+  EXPECT_THAT(Index->Symbols,
+              Contains(AllOf(QName("A"), HasRole(SymbolRole::Reference),
+                             Not(HasRole(SymbolRole::RelationBaseOf)))));
+}
+
 } // namespace
 } // namespace index
 } // namespace clang