From 916b92228ab7d5d1510bbc868acfc6d1db667d85 Mon Sep 17 00:00:00 2001 From: hjk Date: Tue, 8 Mar 2016 15:03:31 +0100 Subject: [PATCH] Valgrind: Use a lambda to provide filtering in the error model Makes code more compact and local. Change-Id: Id8973558292257e4d8a5a2648cd93f54411984a8 Reviewed-by: Christian Stenger --- src/plugins/valgrind/memchecktool.cpp | 81 ++++++------------- .../valgrind/xmlprotocol/errorlistmodel.cpp | 61 ++++++-------- .../valgrind/xmlprotocol/errorlistmodel.h | 25 +++--- 3 files changed, 60 insertions(+), 107 deletions(-) diff --git a/src/plugins/valgrind/memchecktool.cpp b/src/plugins/valgrind/memchecktool.cpp index 412418d2023..95314de3170 100644 --- a/src/plugins/valgrind/memchecktool.cpp +++ b/src/plugins/valgrind/memchecktool.cpp @@ -89,15 +89,14 @@ using namespace Debugger; using namespace ProjectExplorer; using namespace Utils; using namespace Valgrind::XmlProtocol; +using namespace std::placeholders; namespace Valgrind { namespace Internal { -class FrameFinder : public ErrorListModel::RelevantFrameFinder +static ErrorListModel::RelevantFrameFinder makeFrameFinder(const QStringList &projectFiles) { -public: - Frame findRelevant(const Error &error) const - { + return [projectFiles](const Error &error) { const QVector stacks = error.stacks(); if (stacks.isEmpty()) return Frame(); @@ -107,14 +106,14 @@ public: return Frame(); //find the first frame belonging to the project - if (!m_projectFiles.isEmpty()) { + if (!projectFiles.isEmpty()) { foreach (const Frame &frame, frames) { if (frame.directory().isEmpty() || frame.fileName().isEmpty()) continue; //filepaths can contain "..", clean them: const QString f = QFileInfo(frame.filePath()).absoluteFilePath(); - if (m_projectFiles.contains(f)) + if (projectFiles.contains(f)) return frame; } } @@ -130,21 +129,12 @@ public: //else fallback to the first frame return frames.first(); - } - void setFiles(const QStringList &files) - { - m_projectFiles = files; - } -private: - QStringList m_projectFiles; -}; + }; +} class MemcheckErrorFilterProxyModel : public QSortFilterProxyModel { -public: - MemcheckErrorFilterProxyModel(QObject *parent = 0); - public: void setAcceptedKinds(const QList &acceptedKinds); void setFilterExternalIssues(bool filter); @@ -152,15 +142,9 @@ public: private: QList m_acceptedKinds; - bool m_filterExternalIssues; + bool m_filterExternalIssues = false; }; -MemcheckErrorFilterProxyModel::MemcheckErrorFilterProxyModel(QObject *parent) - : QSortFilterProxyModel(parent), - m_filterExternalIssues(false) -{ -} - void MemcheckErrorFilterProxyModel::setAcceptedKinds(const QList &acceptedKinds) { if (m_acceptedKinds != acceptedKinds) { @@ -275,12 +259,11 @@ private: private: ValgrindBaseSettings *m_settings; - QMenu *m_filterMenu; + QMenu *m_filterMenu = 0; - FrameFinder *m_frameFinder; - Valgrind::XmlProtocol::ErrorListModel *m_errorModel; - MemcheckErrorFilterProxyModel *m_errorProxyModel; - MemcheckErrorView *m_errorView; + Valgrind::XmlProtocol::ErrorListModel m_errorModel; + MemcheckErrorFilterProxyModel m_errorProxyModel; + MemcheckErrorView *m_errorView = 0; QList m_errorFilterActions; QAction *m_filterProjectAction; @@ -299,10 +282,6 @@ MemcheckTool::MemcheckTool(QObject *parent) : QObject(parent) { m_settings = ValgrindPlugin::globalSettings(); - m_errorModel = 0; - m_errorProxyModel = 0; - m_errorView = 0; - m_filterMenu = 0; setObjectName(QLatin1String("MemcheckTool")); @@ -334,22 +313,14 @@ MemcheckTool::MemcheckTool(QObject *parent) initKindFilterAction(a, { InvalidFree, MismatchedFree }); m_errorFilterActions.append(a); - - using namespace std::placeholders; - - QTC_ASSERT(!m_errorView, return); - m_errorView = new MemcheckErrorView; m_errorView->setObjectName(QLatin1String("MemcheckErrorView")); m_errorView->setFrameStyle(QFrame::NoFrame); m_errorView->setAttribute(Qt::WA_MacShowFocusRect, false); - m_errorModel = new ErrorListModel(m_errorView); - m_frameFinder = new Internal::FrameFinder; - m_errorModel->setRelevantFrameFinder(QSharedPointer(m_frameFinder)); - m_errorProxyModel = new MemcheckErrorFilterProxyModel(m_errorView); - m_errorProxyModel->setSourceModel(m_errorModel); - m_errorProxyModel->setDynamicSortFilter(true); - m_errorView->setModel(m_errorProxyModel); + m_errorModel.setRelevantFrameFinder(makeFrameFinder(QStringList())); + m_errorProxyModel.setSourceModel(&m_errorModel); + m_errorProxyModel.setDynamicSortFilter(true); + m_errorView->setModel(&m_errorProxyModel); m_errorView->setSelectionMode(QAbstractItemView::ExtendedSelection); // make m_errorView->selectionModel()->selectedRows() return something m_errorView->setSelectionBehavior(QAbstractItemView::SelectRows); @@ -513,12 +484,12 @@ void MemcheckTool::updateFromSettings() m_errorView->settingsChanged(m_settings); connect(m_settings, &ValgrindBaseSettings::visibleErrorKindsChanged, - m_errorProxyModel, &MemcheckErrorFilterProxyModel::setAcceptedKinds); - m_errorProxyModel->setAcceptedKinds(m_settings->visibleErrorKinds()); + &m_errorProxyModel, &MemcheckErrorFilterProxyModel::setAcceptedKinds); + m_errorProxyModel.setAcceptedKinds(m_settings->visibleErrorKinds()); connect(m_settings, &ValgrindBaseSettings::filterExternalIssuesChanged, - m_errorProxyModel, &MemcheckErrorFilterProxyModel::setFilterExternalIssues); - m_errorProxyModel->setFilterExternalIssues(m_settings->filterExternalIssues()); + &m_errorProxyModel, &MemcheckErrorFilterProxyModel::setFilterExternalIssues); + m_errorProxyModel.setFilterExternalIssues(m_settings->filterExternalIssues()); } void MemcheckTool::maybeActiveRunConfigurationChanged() @@ -541,7 +512,7 @@ void MemcheckTool::maybeActiveRunConfigurationChanged() // disconnect old settings class if any if (m_settings) { m_settings->disconnect(this); - m_settings->disconnect(m_errorProxyModel); + m_settings->disconnect(&m_errorProxyModel); } // now make the new settings current, update and connect input widgets @@ -555,8 +526,8 @@ void MemcheckTool::maybeActiveRunConfigurationChanged() MemcheckRunControl *MemcheckTool::createRunControl(RunConfiguration *runConfiguration, Core::Id runMode) { - m_frameFinder->setFiles(runConfiguration ? runConfiguration->target() - ->project()->files(Project::AllFiles) : QStringList()); + m_errorModel.setRelevantFrameFinder(makeFrameFinder(runConfiguration + ? runConfiguration->target()->project()->files(Project::AllFiles) : QStringList())); MemcheckRunControl *runControl = 0; if (runMode == MEMCHECK_RUN_MODE) @@ -640,7 +611,7 @@ void MemcheckTool::loadExternalXmlLogFile() void MemcheckTool::parserError(const Error &error) { - m_errorModel->addError(error); + m_errorModel.addError(error); } void MemcheckTool::internalParserError(const QString &errorString) @@ -652,7 +623,7 @@ void MemcheckTool::internalParserError(const QString &errorString) void MemcheckTool::clearErrorView() { QTC_ASSERT(m_errorView, return); - m_errorModel->clear(); + m_errorModel.clear(); qDeleteAll(m_suppressionActions); m_suppressionActions.clear(); @@ -682,7 +653,7 @@ void MemcheckTool::updateErrorFilter() int MemcheckTool::updateUiAfterFinishedHelper() { - const int issuesFound = m_errorModel->rowCount(); + const int issuesFound = m_errorModel.rowCount(); m_goBack->setEnabled(issuesFound > 1); m_goNext->setEnabled(issuesFound > 1); m_loadExternalLogFile->setEnabled(true); diff --git a/src/plugins/valgrind/xmlprotocol/errorlistmodel.cpp b/src/plugins/valgrind/xmlprotocol/errorlistmodel.cpp index 915b0ab4e88..a5f0a765ddb 100644 --- a/src/plugins/valgrind/xmlprotocol/errorlistmodel.cpp +++ b/src/plugins/valgrind/xmlprotocol/errorlistmodel.cpp @@ -42,27 +42,18 @@ namespace Valgrind { namespace XmlProtocol { -class ErrorListModelPrivate -{ -public: - QVariant errorData(const QModelIndex &index, int role) const; - QSharedPointer relevantFrameFinder; - Frame findRelevantFrame(const Error &error) const; - QString errorLocation(const Error &error) const; -}; - class ErrorItem : public Utils::TreeItem { public: - ErrorItem(const ErrorListModelPrivate *modelPrivate, const Error &error); + ErrorItem(const ErrorListModel *model, const Error &error); - const ErrorListModelPrivate *modelPrivate() const { return m_modelPrivate; } + const ErrorListModel *modelPrivate() const { return m_model; } Error error() const { return m_error; } private: QVariant data(int column, int role) const override; - const ErrorListModelPrivate * const m_modelPrivate; + const ErrorListModel * const m_model; const Error m_error; }; @@ -95,30 +86,14 @@ private: ErrorListModel::ErrorListModel(QObject *parent) : Utils::TreeModel(parent) - , d(new ErrorListModelPrivate) { setHeader(QStringList() << tr("Issue") << tr("Location")); } -ErrorListModel::~ErrorListModel() +Frame ErrorListModel::findRelevantFrame(const Error &error) const { - delete d; -} - -QSharedPointer ErrorListModel::relevantFrameFinder() const -{ - return d->relevantFrameFinder; -} - -void ErrorListModel::setRelevantFrameFinder(const QSharedPointer &finder) -{ - d->relevantFrameFinder = finder; -} - -Frame ErrorListModelPrivate::findRelevantFrame(const Error &error) const -{ - if (relevantFrameFinder) - return relevantFrameFinder->findRelevant(error); + if (m_relevantFrameFinder) + return m_relevantFrameFinder(error); const QVector stacks = error.stacks(); if (stacks.isEmpty()) return Frame(); @@ -158,7 +133,7 @@ static QString makeFrameName(const Frame &frame, bool withLocation) return QString::fromLatin1("0x%1").arg(frame.instructionPointer(), 0, 16); } -QString ErrorListModelPrivate::errorLocation(const Error &error) const +QString ErrorListModel::errorLocation(const Error &error) const { return QCoreApplication::translate("Valgrind::Internal", "in %1"). arg(makeFrameName(findRelevantFrame(error), true)); @@ -166,12 +141,22 @@ QString ErrorListModelPrivate::errorLocation(const Error &error) const void ErrorListModel::addError(const Error &error) { - rootItem()->appendChild(new ErrorItem(d, error)); + rootItem()->appendChild(new ErrorItem(this, error)); +} + +ErrorListModel::RelevantFrameFinder ErrorListModel::relevantFrameFinder() const +{ + return m_relevantFrameFinder; +} + +void ErrorListModel::setRelevantFrameFinder(const RelevantFrameFinder &relevantFrameFinder) +{ + m_relevantFrameFinder = relevantFrameFinder; } -ErrorItem::ErrorItem(const ErrorListModelPrivate *modelPrivate, const Error &error) - : m_modelPrivate(modelPrivate), m_error(error) +ErrorItem::ErrorItem(const ErrorListModel *model, const Error &error) + : m_model(model), m_error(error) { QTC_ASSERT(!m_error.stacks().isEmpty(), return); @@ -203,7 +188,7 @@ static QVariant location(const Frame &frame, int role) QVariant ErrorItem::data(int column, int role) const { if (column == Debugger::DetailedErrorView::LocationColumn) { - const Frame frame = m_modelPrivate->findRelevantFrame(m_error); + const Frame frame = m_model->findRelevantFrame(m_error); return location(frame, role); } @@ -215,7 +200,7 @@ QVariant ErrorItem::data(int column, int role) const stream << m_error.what() << "\n"; stream << " " - << m_modelPrivate->errorLocation(m_error) + << m_model->errorLocation(m_error) << "\n"; foreach (const Stack &stack, m_error.stacks()) { @@ -241,7 +226,7 @@ QVariant ErrorItem::data(int column, int role) const return ErrorListModel::tr("%1 in function %2") .arg(m_error.what(), m_error.stacks().first().frames().first().functionName()); case Qt::ToolTipRole: - return toolTipForFrame(m_modelPrivate->findRelevantFrame(m_error)); + return toolTipForFrame(m_model->findRelevantFrame(m_error)); default: return QVariant(); } diff --git a/src/plugins/valgrind/xmlprotocol/errorlistmodel.h b/src/plugins/valgrind/xmlprotocol/errorlistmodel.h index 020c08dd4bb..6112d152a86 100644 --- a/src/plugins/valgrind/xmlprotocol/errorlistmodel.h +++ b/src/plugins/valgrind/xmlprotocol/errorlistmodel.h @@ -29,13 +29,12 @@ #include #include -#include +#include namespace Valgrind { namespace XmlProtocol { class Error; -class ErrorListModelPrivate; class Frame; class ErrorListModel : public Utils::TreeModel @@ -47,24 +46,22 @@ public: ErrorRole = Debugger::DetailedErrorView::FullTextRole + 1, }; - class RelevantFrameFinder - { - public: - virtual ~RelevantFrameFinder() {} - virtual Frame findRelevant(const Error &error) const = 0; - }; - explicit ErrorListModel(QObject *parent = 0); - ~ErrorListModel(); - QSharedPointer relevantFrameFinder() const; - void setRelevantFrameFinder(const QSharedPointer &finder); + typedef std::function RelevantFrameFinder; + RelevantFrameFinder relevantFrameFinder() const; + void setRelevantFrameFinder(const RelevantFrameFinder &relevantFrameFinder); -public slots: void addError(const Valgrind::XmlProtocol::Error &error); private: - ErrorListModelPrivate *const d; + friend class ErrorItem; + friend class StackItem; + QVariant errorData(const QModelIndex &index, int role) const; + Frame findRelevantFrame(const Error &error) const; + QString errorLocation(const Error &error) const; + + RelevantFrameFinder m_relevantFrameFinder; }; } // namespace XmlProtocol