diff --git a/src/plugins/coreplugin/locator/ilocatorfilter.cpp b/src/plugins/coreplugin/locator/ilocatorfilter.cpp index 9628fc49fde..bb4795fcc16 100644 --- a/src/plugins/coreplugin/locator/ilocatorfilter.cpp +++ b/src/plugins/coreplugin/locator/ilocatorfilter.cpp @@ -93,13 +93,16 @@ public: , m_outputData(filterCount, {}) {} - void addOutputData(int index, const LocatorFilterEntries &outputData) + void reportOutput(int index, const LocatorFilterEntries &outputData) { QTC_ASSERT(index >= 0, return); QMutexLocker locker(&m_mutex); + // It may happen that the task tree was canceled, while tasks are still running in other + // threads and are about to be canceled. In this case we just ignore the call. + if (m_state == State::Canceled) + return; QTC_ASSERT(index < m_filterCount, return); - QTC_ASSERT(m_state != State::Canceled, return); QTC_ASSERT(!m_outputData.at(index).has_value(), return); m_outputData[index] = outputData; @@ -201,12 +204,12 @@ class OutputFilter : public QObject public: ~OutputFilter(); void setFilterCount(int count); - // When last index is added it ends automatically (asynchronously) - void addOutputData(int index, const LocatorFilterEntries &outputData); void start(); bool isRunning() const { return m_watcher.get(); } + std::shared_ptr dataProvider() const { return m_dataProvider; } + signals: void serialOutputDataReady(const LocatorFilterEntries &serialOutputData); void done(); @@ -234,13 +237,6 @@ void OutputFilter::setFilterCount(int count) m_filterCount = count; } -void OutputFilter::addOutputData(int index, const LocatorFilterEntries &outputData) -{ - QTC_ASSERT(isRunning(), return); - - m_dataProvider->addOutputData(index, outputData); -} - void OutputFilter::start() { QTC_ASSERT(!m_watcher, return); @@ -284,11 +280,72 @@ QTC_DECLARE_CUSTOM_TASK(Filter, Core::OutputFilterAdapter); namespace Core { +class LocatorStoragePrivate +{ +public: + LocatorStoragePrivate(const QString &input, int index, + const std::shared_ptr &dataProvider) + : m_input(input) + , m_index(index) + , m_dataProvider(dataProvider) + {} + + QString input() const { return m_input; } + + void reportOutput(const LocatorFilterEntries &outputData) + { + QMutexLocker locker(&m_mutex); + QTC_ASSERT(m_dataProvider, return); + reportOutputImpl(outputData); + } + + void finalize() + { + QMutexLocker locker(&m_mutex); + if (m_dataProvider) + reportOutputImpl({}); + } + +private: + // Call me with mutex locked + void reportOutputImpl(const LocatorFilterEntries &outputData) + { + QTC_ASSERT(m_index >= 0, return); + m_dataProvider->reportOutput(m_index, outputData); + // Deliver results only once for all copies of the storage, drop ref afterwards + m_dataProvider.reset(); + } + + const QString m_input; + const int m_index = -1; + std::shared_ptr m_dataProvider; + QMutex m_mutex = {}; +}; + +QString LocatorStorage::input() const +{ + QTC_ASSERT(d, return {}); + return d->input(); +} + +void LocatorStorage::reportOutput(const LocatorFilterEntries &outputData) const +{ + QTC_ASSERT(d, return); + d->reportOutput(outputData); +} + +void LocatorStorage::finalize() const +{ + QTC_ASSERT(d, return); + d->finalize(); +} + class LocatorMatcherPrivate { public: LocatorMatcherTasks m_tasks; - LocatorMatcherTask::Storage m_storage; + QString m_input; + LocatorFilterEntries m_output; int m_parallelLimit = 0; std::unique_ptr m_taskTree; }; @@ -305,7 +362,7 @@ void LocatorMatcher::setTasks(const LocatorMatcherTasks &tasks) void LocatorMatcher::setInputData(const QString &inputData) { - d->m_storage.input = inputData; + d->m_input = inputData; } void LocatorMatcher::setParallelLimit(int limit) @@ -316,7 +373,7 @@ void LocatorMatcher::setParallelLimit(int limit) void LocatorMatcher::start() { QTC_ASSERT(!isRunning(), return); - d->m_storage.output = {}; + d->m_output = {}; d->m_taskTree.reset(new TaskTree); using namespace Tasking; @@ -332,7 +389,7 @@ void LocatorMatcher::start() filter.setFilterCount(filterCount); connect(&filter, &OutputFilter::serialOutputDataReady, this, [this](const LocatorFilterEntries &serialOutputData) { - d->m_storage.output += serialOutputData; + d->m_output += serialOutputData; emit serialOutputDataReady(serialOutputData); }); }; @@ -343,27 +400,29 @@ void LocatorMatcher::start() QList parallelTasks { ParallelLimit(d->m_parallelLimit) }; - const auto onGroupSetup = [this](const TreeStorage &storage) { - return [this, storage] { storage->input = d->m_storage.input; }; - }; - const auto onGroupDone = [filterStorage] - (const TreeStorage &storage, int index) { - return [filterStorage, storage, index] { + const auto onGroupSetup = [this, filterStorage](const TreeStorage &storage, + int index) { + return [this, filterStorage, storage, index] { OutputFilter *outputFilter = filterStorage->m_filter; QTC_ASSERT(outputFilter, return); - outputFilter->addOutputData(index, storage->output); + *storage = std::make_shared(d->m_input, index, + outputFilter->dataProvider()); }; }; + const auto onGroupDone = [](const TreeStorage &storage) { + return [storage] { storage->finalize(); }; + }; + int index = 0; for (const LocatorMatcherTask &task : std::as_const(d->m_tasks)) { const auto storage = task.storage; const Group group { optional, Storage(storage), - OnGroupSetup(onGroupSetup(storage)), - OnGroupDone(onGroupDone(storage, index)), - OnGroupError(onGroupDone(storage, index)), + OnGroupSetup(onGroupSetup(storage, index)), + OnGroupDone(onGroupDone(storage)), + OnGroupError(onGroupDone(storage)), task.task }; parallelTasks << group; @@ -408,7 +467,7 @@ bool LocatorMatcher::isRunning() const LocatorFilterEntries LocatorMatcher::outputData() const { - return d->m_storage.output; + return d->m_output; } LocatorFilterEntries LocatorMatcher::runBlocking(const LocatorMatcherTasks &tasks, diff --git a/src/plugins/coreplugin/locator/ilocatorfilter.h b/src/plugins/coreplugin/locator/ilocatorfilter.h index e89bdc0885c..e2866d17519 100644 --- a/src/plugins/coreplugin/locator/ilocatorfilter.h +++ b/src/plugins/coreplugin/locator/ilocatorfilter.h @@ -25,6 +25,7 @@ namespace Core { namespace Internal { class Locator; } class ILocatorFilter; +class LocatorStoragePrivate; class AcceptResult { @@ -114,22 +115,29 @@ public: using LocatorFilterEntries = QList; +class CORE_EXPORT LocatorStorage final +{ +public: + LocatorStorage() = default; + QString input() const; + void reportOutput(const LocatorFilterEntries &outputData) const; + +private: + friend class LocatorMatcher; + LocatorStorage(const std::shared_ptr &priv) { d = priv; } + void finalize() const; + std::shared_ptr d; +}; + class CORE_EXPORT LocatorMatcherTask final { public: - class Storage - { - public: - QString input; - LocatorFilterEntries output; - }; - // The main task. Initial data taken from storage.input field. - // Results reporting is done through the storage.output field. + // The main task. Initial data (searchTerm) should be taken from storage.input(). + // Results reporting is done via the storage.reportOutput(). Utils::Tasking::TaskItem task = Utils::Tasking::Group{}; - // When setting up the task, take the input data from storage.input field. - // When task is done, report results by updating the storage.output field. + // When constructing the task, don't place the storage inside the task above. - Utils::Tasking::TreeStorage storage; + Utils::Tasking::TreeStorage storage; }; using LocatorMatcherTasks = QList; @@ -267,6 +275,9 @@ protected: static bool isOldSetting(const QByteArray &state); private: + // TODO: Make pure virtual when all subclasses implement it. + virtual LocatorMatcherTasks matchers() { return {}; } + friend class Internal::Locator; static const QList allLocatorFilters(); diff --git a/src/plugins/cppeditor/cpplocatorfilter.cpp b/src/plugins/cppeditor/cpplocatorfilter.cpp index 13320c2ae62..0435f03c9ae 100644 --- a/src/plugins/cppeditor/cpplocatorfilter.cpp +++ b/src/plugins/cppeditor/cpplocatorfilter.cpp @@ -26,18 +26,19 @@ namespace CppEditor { using EntryFromIndex = std::function; -void matchesFor(QPromise &promise, const QString &entry, +void matchesFor(QPromise &promise, const LocatorStorage &storage, IndexItem::ItemType wantedType, const EntryFromIndex &converter) { + const QString input = storage.input(); LocatorFilterEntries entries[int(ILocatorFilter::MatchLevel::Count)]; - const Qt::CaseSensitivity caseSensitivityForPrefix = ILocatorFilter::caseSensitivity(entry); - const QRegularExpression regexp = ILocatorFilter::createRegExp(entry); + const Qt::CaseSensitivity caseSensitivityForPrefix = ILocatorFilter::caseSensitivity(input); + const QRegularExpression regexp = ILocatorFilter::createRegExp(input); if (!regexp.isValid()) return; - const bool hasColonColon = entry.contains("::"); + const bool hasColonColon = input.contains("::"); const QRegularExpression shortRegexp = hasColonColon - ? ILocatorFilter::createRegExp(entry.mid(entry.lastIndexOf("::") + 2)) : regexp; + ? ILocatorFilter::createRegExp(input.mid(input.lastIndexOf("::") + 2)) : regexp; CppLocatorData *locatorData = CppModelManager::instance()->locatorData(); locatorData->filterAllFiles([&](const IndexItem::Ptr &info) { if (promise.isCanceled()) @@ -76,9 +77,9 @@ void matchesFor(QPromise &promise, const QString &entry, if (matchInParameterList) entries[int(ILocatorFilter::MatchLevel::Normal)].append(filterEntry); - else if (filterEntry.displayName.startsWith(entry, caseSensitivityForPrefix)) + else if (filterEntry.displayName.startsWith(input, caseSensitivityForPrefix)) entries[int(ILocatorFilter::MatchLevel::Best)].append(filterEntry); - else if (filterEntry.displayName.contains(entry, caseSensitivityForPrefix)) + else if (filterEntry.displayName.contains(input, caseSensitivityForPrefix)) entries[int(ILocatorFilter::MatchLevel::Better)].append(filterEntry); else entries[int(ILocatorFilter::MatchLevel::Good)].append(filterEntry); @@ -95,7 +96,7 @@ void matchesFor(QPromise &promise, const QString &entry, Utils::sort(entry, LocatorFilterEntry::compareLexigraphically); } - promise.addResult(std::accumulate(std::begin(entries), std::end(entries), + storage.reportOutput(std::accumulate(std::begin(entries), std::end(entries), LocatorFilterEntries())); } @@ -103,17 +104,13 @@ LocatorMatcherTask locatorMatcher(IndexItem::ItemType type, const EntryFromIndex { using namespace Tasking; - TreeStorage storage; + TreeStorage storage; - const auto onSetup = [=](AsyncTask &async) { + const auto onSetup = [=](AsyncTask &async) { async.setFutureSynchronizer(Internal::CppEditorPlugin::futureSynchronizer()); - async.setConcurrentCallData(matchesFor, storage->input, type, converter); + async.setConcurrentCallData(matchesFor, *storage, type, converter); }; - const auto onDone = [storage](const AsyncTask &async) { - if (async.isResultAvailable()) - storage->output = async.result(); - }; - return {Async(onSetup, onDone, onDone), storage}; + return {Async(onSetup), storage}; } LocatorMatcherTask cppAllSymbolsMatcher() @@ -200,10 +197,11 @@ LocatorFilterEntry::HighlightInfo highlightInfo(const QRegularExpressionMatch &m return LocatorFilterEntry::HighlightInfo(positions.starts, positions.lengths, dataType); } -void matchesForCurrentDocument(QPromise &promise, - const QString &entry, const FilePath ¤tFileName) +void matchesForCurrentDocument(QPromise &promise, const LocatorStorage &storage, + const FilePath ¤tFileName) { - const QRegularExpression regexp = FuzzyMatcher::createRegExp(entry, Qt::CaseInsensitive, false); + const QString input = storage.input(); + const QRegularExpression regexp = FuzzyMatcher::createRegExp(input, Qt::CaseInsensitive, false); if (!regexp.isValid()) return; @@ -288,7 +286,7 @@ void matchesForCurrentDocument(QPromise &promise, } } } - promise.addResult(Utils::transform(betterEntries, + storage.reportOutput(Utils::transform(betterEntries, [](const Entry &entry) { return entry.entry; })); } @@ -302,17 +300,13 @@ LocatorMatcherTask cppCurrentDocumentMatcher() { using namespace Tasking; - TreeStorage storage; + TreeStorage storage; - const auto onSetup = [=](AsyncTask &async) { + const auto onSetup = [=](AsyncTask &async) { async.setFutureSynchronizer(Internal::CppEditorPlugin::futureSynchronizer()); - async.setConcurrentCallData(matchesForCurrentDocument, storage->input, currentFileName()); + async.setConcurrentCallData(matchesForCurrentDocument, *storage, currentFileName()); }; - const auto onDone = [storage](const AsyncTask &async) { - if (async.isResultAvailable()) - storage->output = async.result(); - }; - return {Async(onSetup, onDone, onDone), storage}; + return {Async(onSetup), storage}; } CppLocatorFilter::CppLocatorFilter() diff --git a/src/plugins/languageclient/locatorfilter.cpp b/src/plugins/languageclient/locatorfilter.cpp index a0229b11869..0fe00c23879 100644 --- a/src/plugins/languageclient/locatorfilter.cpp +++ b/src/plugins/languageclient/locatorfilter.cpp @@ -28,12 +28,14 @@ using namespace Utils; namespace LanguageClient { -void filterResults(QPromise &promise, Client *client, +void filterResults(QPromise &promise, const LocatorStorage &storage, Client *client, const QList &results, const QList &filter) { const auto doFilter = [&](const SymbolInformation &info) { return filter.contains(SymbolKind(info.kind())); }; + if (promise.isCanceled()) + return; const QList filteredResults = filter.isEmpty() ? results : Utils::filtered(results, doFilter); const auto generateEntry = [client](const SymbolInformation &info) { @@ -45,7 +47,7 @@ void filterResults(QPromise &promise, Client *client, entry.linkForEditor = info.location().toLink(client->hostPathMapper()); return entry; }; - promise.addResult(Utils::transform(filteredResults, generateEntry)); + storage.reportOutput(Utils::transform(filteredResults, generateEntry)); } LocatorMatcherTask locatorMatcher(Client *client, int maxResultCount, @@ -53,13 +55,13 @@ LocatorMatcherTask locatorMatcher(Client *client, int maxResultCount, { using namespace Tasking; - TreeStorage storage; + TreeStorage storage; TreeStorage> resultStorage; const auto onQuerySetup = [=](WorkspaceSymbolRequestTask &request) { request.setClient(client); WorkspaceSymbolParams params; - params.setQuery(storage->input); + params.setQuery(storage->input()); if (maxResultCount > 0) params.setLimit(maxResultCount); request.setParams(params); @@ -71,23 +73,19 @@ LocatorMatcherTask locatorMatcher(Client *client, int maxResultCount, *resultStorage = result->toList(); }; - const auto onFilterSetup = [=](AsyncTask &async) { + const auto onFilterSetup = [=](AsyncTask &async) { const QList results = *resultStorage; if (results.isEmpty()) return TaskAction::StopWithDone; async.setFutureSynchronizer(LanguageClientPlugin::futureSynchronizer()); - async.setConcurrentCallData(filterResults, client, results, filter); + async.setConcurrentCallData(filterResults, *storage, client, results, filter); return TaskAction::Continue; }; - const auto onFilterDone = [storage](const AsyncTask &async) { - if (async.isResultAvailable()) - storage->output = async.result(); - }; const Group root { Storage(resultStorage), SymbolRequest(onQuerySetup, onQueryDone), - Async(onFilterSetup, onFilterDone) + Async(onFilterSetup) }; return {root, storage}; }