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.
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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
