diff options
author | Christian Kandeler <christian.kandeler@qt.io> | 2021-05-28 13:12:00 +0200 |
---|---|---|
committer | Christian Kandeler <christian.kandeler@qt.io> | 2021-06-11 13:57:01 +0000 |
commit | 7275402ce9ac56d7a208aaea1fe1e99ca01c43a2 (patch) | |
tree | 1a6dbc325a219946a052d3153111ac3a200c9cbe /src | |
parent | ed426f041d03dbecde512f3b51b3ce634d9c05a1 (diff) |
ClangCodeModel: Fix links for virtual overrides
For some reason, clangd returns the declaration instead of the definition
position in the "Goto Implementation" result, so we have to do another
look-up for each override.
Change-Id: I2a99eb0dacdea07d5882087445dc2b2d61b24e58
Reviewed-by: David Schulz <david.schulz@qt.io>
Diffstat (limited to 'src')
-rw-r--r-- | src/libs/utils/link.cpp | 7 | ||||
-rw-r--r-- | src/libs/utils/link.h | 3 | ||||
-rw-r--r-- | src/plugins/clangcodemodel/clangdclient.cpp | 66 | ||||
-rw-r--r-- | src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp | 8 |
4 files changed, 70 insertions, 14 deletions
diff --git a/src/libs/utils/link.cpp b/src/libs/utils/link.cpp index 6197ce4f93c..923f6697993 100644 --- a/src/libs/utils/link.cpp +++ b/src/libs/utils/link.cpp @@ -51,4 +51,11 @@ Link Link::fromString(const QString &fileName, bool canContainLineNumber, QStrin lineColumn.column}; } +uint qHash(const Link &l) +{ + QString s = l.targetFilePath.toString(); + return qHash(s.append(':').append(QString::number(l.targetLine)).append(':') + .append(QString::number(l.targetColumn))); +} + } // namespace Utils diff --git a/src/libs/utils/link.h b/src/libs/utils/link.h index 21d87d7aafd..1d16d426102 100644 --- a/src/libs/utils/link.h +++ b/src/libs/utils/link.h @@ -62,6 +62,7 @@ public: && linkTextStart == other.linkTextStart && linkTextEnd == other.linkTextEnd; } + bool operator!=(const Link &other) const { return !(*this == other); } int linkTextStart = -1; int linkTextEnd = -1; @@ -71,6 +72,8 @@ public: int targetColumn; }; +uint QTCREATOR_UTILS_EXPORT qHash(const Link &l); + using ProcessLinkCallback = std::function<void(const Link &)>; } // namespace Utils diff --git a/src/plugins/clangcodemodel/clangdclient.cpp b/src/plugins/clangcodemodel/clangdclient.cpp index e9cd86d9a68..f39df0aeeb8 100644 --- a/src/plugins/clangcodemodel/clangdclient.cpp +++ b/src/plugins/clangcodemodel/clangdclient.cpp @@ -465,6 +465,8 @@ public: q->cancelRequest(id); for (const MessageId &id : qAsConst(pendingGotoImplRequests)) q->cancelRequest(id); + for (const MessageId &id : qAsConst(pendingGotoDefRequests)) + q->cancelRequest(id); } void closeTempDocuments() @@ -492,10 +494,12 @@ public: VirtualFunctionAssistProvider virtualFuncAssistProvider; QList<MessageId> pendingSymbolInfoRequests; QList<MessageId> pendingGotoImplRequests; + QList<MessageId> pendingGotoDefRequests; const bool openInSplit; Utils::Link defLink; QList<Utils::Link> allLinks; + QHash<Utils::Link, Utils::Link> declDefMap; AstNode cursorNode; AstNode defLinkNode; SymbolDataList symbolsToDisplay; @@ -581,6 +585,7 @@ ClangdClient::~ClangdClient() d->followSymbolData->openedFiles.clear(); d->followSymbolData->pendingSymbolInfoRequests.clear(); d->followSymbolData->pendingGotoImplRequests.clear(); + d->followSymbolData->pendingGotoDefRequests.clear(); } delete d; } @@ -1015,6 +1020,8 @@ void ClangdClient::Private::handleGotoImplementationResult( // Make a symbol info request for each link to get the class names. // Also get the AST for the base declaration, so we can find out whether it's // pure virtual and mark it accordingly. + // In addition, we need to follow all override links, because for these, clangd + // gives us the declaration instead of the definition. for (const Utils::Link &link : qAsConst(followSymbolData->allLinks)) { if (!q->documentForFilePath(link.targetFilePath) && followSymbolData->openedFiles.insert(link.targetFilePath).second) { @@ -1022,8 +1029,9 @@ void ClangdClient::Private::handleGotoImplementationResult( } const TextDocumentIdentifier doc(DocumentUri::fromFilePath(link.targetFilePath)); const Position pos(link.targetLine - 1, link.targetColumn); - SymbolInfoRequest req(TextDocumentPositionParams(doc, pos)); - req.setResponseCallback([this, link, id = followSymbolData->id, reqId = req.id()]( + const TextDocumentPositionParams params(doc, pos); + SymbolInfoRequest symReq(params); + symReq.setResponseCallback([this, link, id = followSymbolData->id, reqId = symReq.id()]( const SymbolInfoRequest::Response &response) { qCDebug(clangdLog) << "handling symbol info reply" << link.targetFilePath.toUserOutput() << link.targetLine; @@ -1043,13 +1051,48 @@ void ClangdClient::Private::handleGotoImplementationResult( } followSymbolData->pendingSymbolInfoRequests.removeOne(reqId); if (followSymbolData->pendingSymbolInfoRequests.isEmpty() + && followSymbolData->pendingGotoDefRequests.isEmpty() && followSymbolData->defLinkNode.isValid()) { handleDocumentInfoResults(); } }); - followSymbolData->pendingSymbolInfoRequests << req.id(); + followSymbolData->pendingSymbolInfoRequests << symReq.id(); qCDebug(clangdLog) << "sending symbol info request"; - q->sendContent(req); + q->sendContent(symReq); + + if (link == followSymbolData->defLink) + continue; + + GotoDefinitionRequest defReq(params); + defReq.setResponseCallback([this, link, id = followSymbolData->id, reqId = defReq.id()] + (const GotoDefinitionRequest::Response &response) { + qCDebug(clangdLog) << "handling additional go to definition reply for" + << link.targetFilePath << link.targetLine; + if (!followSymbolData || id != followSymbolData->id) + return; + Utils::Link newLink; + if (Utils::optional<GotoResult> _result = response.result()) { + const GotoResult result = _result.value(); + if (const auto ploc = Utils::get_if<Location>(&result)) { + newLink = ploc->toLink(); + } else if (const auto plloc = Utils::get_if<QList<Location>>(&result)) { + if (!plloc->isEmpty()) + newLink = plloc->value(0).toLink(); + } + } + qCDebug(clangdLog) << "def link is" << newLink.targetFilePath << newLink.targetLine; + followSymbolData->declDefMap.insert(link, newLink); + followSymbolData->pendingGotoDefRequests.removeOne(reqId); + if (followSymbolData->pendingSymbolInfoRequests.isEmpty() + && followSymbolData->pendingGotoDefRequests.isEmpty() + && followSymbolData->defLinkNode.isValid()) { + handleDocumentInfoResults(); + } + }); + followSymbolData->pendingGotoDefRequests << defReq.id(); + qCDebug(clangdLog) << "sending additional go to definition request" + << link.targetFilePath << link.targetLine; + q->sendContent(defReq); } const DocumentUri defLinkUri @@ -1066,8 +1109,10 @@ void ClangdClient::Private::handleGotoImplementationResult( const auto result = response.result(); if (result) followSymbolData->defLinkNode = *result; - if (followSymbolData->pendingSymbolInfoRequests.isEmpty()) + if (followSymbolData->pendingSymbolInfoRequests.isEmpty() + && followSymbolData->pendingGotoDefRequests.isEmpty()) { handleDocumentInfoResults(); + } }); qCDebug(clangdLog) << "sending ast request for def link"; q->sendContent(astRequest); @@ -1107,10 +1152,17 @@ void ClangdClient::VirtualFunctionAssistProcessor::finalize() { QList<TextEditor::AssistProposalItemInterface *> items; for (const SymbolData &symbol : qAsConst(m_data->followSymbolData->symbolsToDisplay)) { + Utils::Link link = symbol.second; + const bool isOriginalLink = m_data->followSymbolData->defLink == symbol.second; + if (!isOriginalLink) { + const Utils::Link defLink = m_data->followSymbolData->declDefMap.value(symbol.second); + if (defLink.hasValidTarget()) + link = defLink; + } const auto item = new CppTools::VirtualFunctionProposalItem( - symbol.second, m_data->followSymbolData->openInSplit); + link, m_data->followSymbolData->openInSplit); QString text = symbol.first; - if (m_data->followSymbolData->defLink == symbol.second + if (isOriginalLink && (m_data->followSymbolData->defLinkNode.isPureVirtualDeclaration() || m_data->followSymbolData->defLinkNode.isPureVirtualDefinition())) { text += " = 0"; diff --git a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp index 32c0f27186b..75f70497472 100644 --- a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp +++ b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp @@ -516,14 +516,8 @@ F2TestCase::F2TestCase(CppEditorAction action, if (useClangd) QEXPECT_FAIL("allOverrides from base declaration", "FIXME: check why this fails", Abort); QCOMPARE(finalVirtualSymbolResults.size(), expectedVirtualFunctionProposal.size()); - if (useClangd) { - QEXPECT_FAIL("allOverrides", "FIXME: clangd sometimes goes to decl instead of def", Abort); - QEXPECT_FAIL("possibleOverrides1", "FIXME: clangd sometimes goes to decl instead of def", - Abort); - QEXPECT_FAIL("possibleOverrides2", "FIXME: clangd sometimes goes to decl instead of def", - Abort); + if (useClangd) QEXPECT_FAIL("itemOrder", "FIXME: sort items", Abort); - } QCOMPARE(finalVirtualSymbolResults, expectedVirtualFunctionProposal); } |