[clangd] Fix a hover crash on templated spaceship operator.

[adamcz]

Summary

We make assumption that:
getDeclForComment(getDeclForComment(X)) == getDeclForComment(X)
but this is not true if you have a template
instantionation of a template instantiation, which is the case when, for
example, you have a <=> operator in a templated class.

This fix makes getDeclForComment() call itself recursively to ensure
this property is always true.

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

[kadircet]

clang-tools-extra/clangd/Hover.cpp

269–270

can we also turn these ifs into else ifs (similarly for ifs at the top level) just to make sure we don't change behaviour (i suppose it should be fine, as the TIP will always be null in the case of undeclared specializations, but better safe than sorry).

285

s/instinstantiation/instantiation (unless you intentionally put a double inst :P)

[adamcz]

review comments

clang-tools-extra/clangd/Hover.cpp

const FunctionDecl *getUnderlyingFunction(const Decl *D) {
 
   // Non-lambda functions.
   return D->getAsFunction();
 }
 
 // Returns the decl that should be used for querying comments, either from index
 // or AST.
 const NamedDecl *getDeclForComment(const NamedDecl *D) {
+  const NamedDecl *DeclForComment = D;
   if (const auto *TSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D)) {
     // Template may not be instantiated e.g. if the type didn't need to be
     // complete; fallback to primary template.
     if (TSD->getTemplateSpecializationKind() == TSK_Undeclared)
-      return TSD->getSpecializedTemplate();
-    if (const auto *TIP = TSD->getTemplateInstantiationPattern())
-      return TIP;
-  }
-  if (const auto *TSD = llvm::dyn_cast<VarTemplateSpecializationDecl>(D)) {
+      DeclForComment = TSD->getSpecializedTemplate();
+    else if (const auto *TIP = TSD->getTemplateInstantiationPattern())
+      DeclForComment = TIP;
+  } else if (const auto *TSD =
+                 llvm::dyn_cast<VarTemplateSpecializationDecl>(D)) {
     if (TSD->getTemplateSpecializationKind() == TSK_Undeclared)
-      return TSD->getSpecializedTemplate();
-    if (const auto *TIP = TSD->getTemplateInstantiationPattern())
-      return TIP;
-  }
-  if (const auto *FD = D->getAsFunction())
+      DeclForComment = TSD->getSpecializedTemplate();
+    else if (const auto *TIP = TSD->getTemplateInstantiationPattern())
+      DeclForComment = TIP;
+  } else if (const auto *FD = D->getAsFunction())
     if (const auto *TIP = FD->getTemplateInstantiationPattern())
-      return TIP;
-  return D;
+      DeclForComment = TIP;
+  // Ensure that getDeclForComment(getDeclForComment(X)) = getDeclForComment(X).
+  // This is usually not needed, but in strange cases of comparision operators
+  // being instantiated from spasceship operater, which itself is a template
+  // instantiation the recursrive call is necessary.
+  if (D != DeclForComment)
+    DeclForComment = getDeclForComment(DeclForComment);
+  return DeclForComment;
 }
 
 // Look up information about D from the index, and add it to Hover.
 void enhanceFromIndex(HoverInfo &Hover, const NamedDecl &ND,
                       const SymbolIndex *Index) {
   assert(&ND == getDeclForComment(&ND));
   // We only add documentation, so don't bother if we already have some.
   if (!Hover.Documentation.empty() || !Index)

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

TEST(Hover, PresentRulers) {
 
   llvm::StringRef ExpectedPlaintext = R"pt(variable foo
 
 Value = val
 
 def)pt";
   EXPECT_EQ(HI.present().asPlainText(), ExpectedPlaintext);
 }
+
+TEST(Hover, SpaceshipTemplateNoCrash) {
+  Annotations T(R"cpp(
+  namespace std {
+  struct strong_ordering {
+    int n;
+    constexpr operator int() const { return n; }
+    static const strong_ordering equal, greater, less;
+  };
+  constexpr strong_ordering strong_ordering::equal = {0};
+  constexpr strong_ordering strong_ordering::greater = {1};
+  constexpr strong_ordering strong_ordering::less = {-1};
+  }
+
+  template <typename T>
+  struct S {
+    // Foo bar baz
+    friend auto operator<=>(S, S) = default;
+  };
+  static_assert(S<void>() =^= S<void>());
+    )cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  TU.ExtraArgs.push_back("-std=c++20");
+  auto AST = TU.build();
+  auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+  EXPECT_EQ(HI->Documentation, "Foo bar baz");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang