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

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz created this revision.
adamcz added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman, yaxunl.
adamcz requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112527

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2934,6 +2934,35 @@
 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
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -262,24 +262,31 @@
 // 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();
+      DeclForComment = TSD->getSpecializedTemplate();
     if (const auto *TIP = TSD->getTemplateInstantiationPattern())
-      return TIP;
+      DeclForComment = TIP;
   }
   if (const auto *TSD = llvm::dyn_cast<VarTemplateSpecializationDecl>(D)) {
     if (TSD->getTemplateSpecializationKind() == TSK_Undeclared)
-      return TSD->getSpecializedTemplate();
+      DeclForComment = TSD->getSpecializedTemplate();
     if (const auto *TIP = TSD->getTemplateInstantiationPattern())
-      return TIP;
+      DeclForComment = TIP;
   }
   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
+  // instinstantiation 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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112527.382265.patch
Type: text/x-patch
Size: 3125 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211026/22d6e341/attachment.bin>