Valgrind: Dismantle Parser class

Replace it with parseDataFile() function.

Change-Id: I21ee1e5ffa2dba94501c7d8e258ac2c5478af633
Reviewed-by: hjk <hjk@qt.io>

src/plugins/valgrind/callgrind/callgrindparser.cpp

@@ -110,17 +110,10 @@ static int parseNameShorthand(const char **current, const char *end)
 }
 
 
-class Parser::Private
+class ParserPrivate
 {
-    Parser *const q;
 public:
-
+    ParseDataPtr parse(const FilePath &filePath);
-    explicit Private(Parser *qq)
-        : q(qq)
-    {
-    }
-
-    void parse(const FilePath &filePath);
     void parseHeader(QIODevice *device);
 
     using NamePair = QPair<qint64, QString>;
@@ -167,7 +160,7 @@ public:
     QSet<Function *> recursiveFunctions;
 };
 
-void Parser::Private::parse(const FilePath &filePath)
+ParseDataPtr ParserPrivate::parse(const FilePath &filePath)
 {
     const QString path = filePath.path(); // FIXME: Works only accidentally for docker
     QFile file(path);
@@ -233,8 +226,7 @@ void Parser::Private::parse(const FilePath &filePath)
     // now accumulate callees
     for (Function *func : std::as_const(pendingFunctions))
         func->finalize();
-
+    return data;
-    emit q->parserDataReady();
 }
 
 inline QString getValue(const QByteArray &line, const int prefixLength)
@@ -244,7 +236,7 @@ inline QString getValue(const QByteArray &line, const int prefixLength)
     return QString::fromLatin1(line.mid(prefixLength, line.length() - 1 - prefixLength).constData());
 }
 
-void Parser::Private::parseHeader(QIODevice *device)
+void ParserPrivate::parseHeader(QIODevice *device)
 {
     QTC_ASSERT(device->isOpen(), return);
     QTC_ASSERT(device->isReadable(), return);
@@ -301,7 +293,7 @@ void Parser::Private::parseHeader(QIODevice *device)
     }
 }
 
-Parser::Private::NamePair Parser::Private::parseName(const char *begin, const char *end)
+ParserPrivate::NamePair ParserPrivate::parseName(const char *begin, const char *end)
 {
     const char *current = begin;
     qint64 nameShorthand = -1;
@@ -325,7 +317,7 @@ Parser::Private::NamePair Parser::Private::parseName(const char *begin, const ch
  * cfn means called function
  */
 
-void Parser::Private::dispatchLine(const QByteArray &line)
+void ParserPrivate::dispatchLine(const QByteArray &line)
 {
     int lineEnding = line.endsWith("\r\n") ? 2 : 1;
     const char *const begin = line.constData();
@@ -408,7 +400,7 @@ void Parser::Private::dispatchLine(const QByteArray &line)
     }
 }
 
-void Parser::Private::parseCostItem(const char *begin, const char *end)
+void ParserPrivate::parseCostItem(const char *begin, const char *end)
 {
     QTC_ASSERT(currentFunction, return);
 
@@ -504,7 +496,7 @@ void Parser::Private::parseCostItem(const char *begin, const char *end)
     currentFunction->addCostItem(costItem);
 }
 
-void Parser::Private::parseSourceFile(const char *begin, const char *end)
+void ParserPrivate::parseSourceFile(const char *begin, const char *end)
 {
     NamePair name = parseName(begin, end);
 
@@ -518,7 +510,7 @@ void Parser::Private::parseSourceFile(const char *begin, const char *end)
     currentDifferingFile = -1;
 }
 
-void Parser::Private::parseFunction(const char *begin, const char *end)
+void ParserPrivate::parseFunction(const char *begin, const char *end)
 {
     currentFunction = new Function(data.get());
     currentFunction->setFile(lastFile);
@@ -534,7 +526,7 @@ void Parser::Private::parseFunction(const char *begin, const char *end)
     currentFunction->setName(name.first);
 }
 
-void Parser::Private::parseDifferingSourceFile(const char *begin, const char *end)
+void ParserPrivate::parseDifferingSourceFile(const char *begin, const char *end)
 {
     NamePair name = parseName(begin, end);
 
@@ -550,7 +542,7 @@ void Parser::Private::parseDifferingSourceFile(const char *begin, const char *en
         currentDifferingFile = name.first;
 }
 
-void Parser::Private::parseObjectFile(const char *begin, const char *end)
+void ParserPrivate::parseObjectFile(const char *begin, const char *end)
 {
     NamePair name = parseName(begin, end);
     if (!name.second.isEmpty())
@@ -559,7 +551,7 @@ void Parser::Private::parseObjectFile(const char *begin, const char *end)
     lastObject = name.first;
 }
 
-void Parser::Private::parseCalls(const char *begin, const char *end)
+void ParserPrivate::parseCalls(const char *begin, const char *end)
 {
     const char *current = begin;
     bool ok;
@@ -577,7 +569,7 @@ void Parser::Private::parseCalls(const char *begin, const char *end)
     isParsingFunctionCall = true;
 }
 
-void Parser::Private::parseCalledFunction(const char *begin, const char *end)
+void ParserPrivate::parseCalledFunction(const char *begin, const char *end)
 {
     NamePair name = parseName(begin, end);
     if (!name.second.isEmpty())
@@ -586,7 +578,7 @@ void Parser::Private::parseCalledFunction(const char *begin, const char *end)
     currentCallData.calledFunction = name.first;
 }
 
-void Parser::Private::parseCalledSourceFile(const char *begin, const char *end)
+void ParserPrivate::parseCalledSourceFile(const char *begin, const char *end)
 {
     NamePair name = parseName(begin, end);
     if (!name.second.isEmpty()) {
@@ -598,7 +590,7 @@ void Parser::Private::parseCalledSourceFile(const char *begin, const char *end)
     currentCallData.calledFile = name.first;
 }
 
-void Parser::Private::parseCalledObjectFile(const char *begin, const char *end)
+void ParserPrivate::parseCalledObjectFile(const char *begin, const char *end)
 {
     NamePair name = parseName(begin, end);
     if (!name.second.isEmpty())
@@ -609,24 +601,10 @@ void Parser::Private::parseCalledObjectFile(const char *begin, const char *end)
 
 //BEGIN Parser
 
-void Parser::parse(const Utils::FilePath &filePath)
+ParseDataPtr parseDataFile(const Utils::FilePath &filePath)
 {
-    d->parse(filePath);
+    ParserPrivate parser;
-}
+    return parser.parse(filePath);
-
-Parser::Parser()
-    : d(new Private(this))
-{
-}
-
-Parser::~Parser()
-{
-    delete d;
-}
-
-ParseDataPtr Parser::parserData() const
-{
-    return d->data;
 }
 
 } // namespace Valgrind::Callgrind

src/plugins/valgrind/callgrind/callgrindparser.h

@@ -11,8 +11,6 @@ namespace Utils { class FilePath; }
 
 namespace Valgrind::Callgrind {
 
-class ParseData;
-
 /**
  * Parser for Valgrind --tool=callgrind output
  * most of the format is documented at http://kcachegrind.sourceforge.net/html/CallgrindFormat.html
@@ -22,25 +20,7 @@ class ParseData;
  * the rest is assumed to be zero."
  *
  */
-class Parser : public QObject
-{
-    Q_OBJECT
 
-public:
+ParseDataPtr parseDataFile(const Utils::FilePath &filePath);
-    Parser();
-    ~Parser() override;
-
-    // get and take ownership of the parsing results. If this function is not called the repository
-    // will be destroyed when the parser is destroyed. Subsequent calls return null.
-    ParseDataPtr parserData() const;
-    void parse(const Utils::FilePath &filePath);
-
-signals:
-    void parserDataReady();
-
-private:
-    class Private;
-    Private *const d;
-};
 
 } // namespace Valgrind::Callgrind

src/plugins/valgrind/callgrindengine.cpp

@@ -34,13 +34,7 @@ CallgrindToolRunner::CallgrindToolRunner(RunControl *runControl)
     connect(&m_runner, &ValgrindProcess::valgrindStarted, this, [this](qint64 pid) {
         m_pid = pid;
     });
-    connect(&m_runner, &ValgrindProcess::done, this, [this] {
+    connect(&m_runner, &ValgrindProcess::done, this, &CallgrindToolRunner::triggerParse);
-        triggerParse();
-        emit parserDataReady(this);
-    });
-    connect(&m_parser, &Callgrind::Parser::parserDataReady, this, [this] {
-        emit parserDataReady(this);
-    });
 
     m_valgrindRunnable = runControl->runnable();
 
@@ -117,11 +111,6 @@ void CallgrindToolRunner::setToggleCollectFunction(const QString &toggleCollectF
     m_argumentForToggleCollect = "--toggle-collect=" + toggleCollectFunction;
 }
 
-Callgrind::ParseDataPtr CallgrindToolRunner::parserData() const
-{
-    return m_parser.parserData();
-}
-
 void CallgrindToolRunner::showStatusMessage(const QString &message)
 {
     Debugger::showPermanentStatusMessage(message);
@@ -253,7 +242,7 @@ void CallgrindToolRunner::triggerParse()
         if (!res) // failed to run callgrind
             return;
         showStatusMessage(Tr::tr("Parsing Profile Data..."));
-        m_parser.parse(m_hostOutputFile);
+        emit parserDataReady(parseDataFile(m_hostOutputFile));
     };
     // TODO: Store the handle and cancel on CallgrindToolRunner destructor?
     // TODO: Should d'tor of context object cancel the running task?

src/plugins/valgrind/callgrindengine.h

@@ -23,8 +23,6 @@ public:
 
     void start() override;
 
-    Callgrind::ParseDataPtr parserData() const;
-
     /// controller actions
     void dump() { run(Dump); }
     void reset() { run(ResetEventCounters); }
@@ -52,7 +50,7 @@ protected:
     QString progressTitle() const override;
 
 signals:
-    void parserDataReady(CallgrindToolRunner *engine);
+    void parserDataReady(const Callgrind::ParseDataPtr &data);
 
 private:
     void showStatusMessage(const QString &message);
@@ -83,7 +81,6 @@ private:
     Utils::FilePath m_valgrindOutputFile; // On the device that runs valgrind
     Utils::FilePath m_hostOutputFile; // On the device that runs creator
 
-    Callgrind::Parser m_parser;
     bool m_paused = false;
 
     QString m_argumentForToggleCollect;

src/plugins/valgrind/callgrindtool.cpp

@@ -137,7 +137,6 @@ public:
     void visualisationFunctionSelected(const Function *function);
     void showParserResults(const ParseDataPtr &data);
 
-    void takeParserDataFromRunControl(CallgrindToolRunner *rc);
     void setParserData(const ParseDataPtr &data);
     void doSetParseData(const ParseDataPtr &data);
     void engineFinished();
@@ -710,7 +709,7 @@ void CallgrindTool::setupRunner(CallgrindToolRunner *toolRunner)
 {
     RunControl *runControl = toolRunner->runControl();
 
-    connect(toolRunner, &CallgrindToolRunner::parserDataReady, this, &CallgrindTool::takeParserDataFromRunControl);
+    connect(toolRunner, &CallgrindToolRunner::parserDataReady, this, &CallgrindTool::setParserData);
     connect(runControl, &RunControl::stopped, this, &CallgrindTool::engineFinished);
 
     connect(this, &CallgrindTool::dumpRequested, toolRunner, &CallgrindToolRunner::dump);
@@ -870,14 +869,7 @@ void CallgrindTool::loadExternalLogFile()
     Debugger::showPermanentStatusMessage(Tr::tr("Parsing Profile Data..."));
     QCoreApplication::processEvents();
 
-    Parser parser;
+    setParserData(parseDataFile(filePath));
-    parser.parse(filePath);
-    setParserData(parser.parserData());
-}
-
-void CallgrindTool::takeParserDataFromRunControl(CallgrindToolRunner *rc)
-{
-    setParserData(rc->parserData());
 }
 
 void CallgrindTool::setParserData(const ParseDataPtr &data)

tests/auto/valgrind/callgrind/callgrindparsertests.cpp

@@ -21,9 +21,10 @@ using namespace Valgrind::Callgrind;
 
 namespace {
 
-static QString dataFile(const char *file)
+static Utils::FilePath dataFile(const char *file)
 {
-    return QLatin1String(PARSERTESTS_DATA_DIR) + QLatin1String("/") + QLatin1String(file);
+    return Utils::FilePath::fromString(
+                QLatin1String(PARSERTESTS_DATA_DIR) + QLatin1String("/") + QLatin1String(file));
 }
 
 void testCostItem(const CostItem *item, quint64 expectedPosition, quint64 expectedCost)
@@ -80,13 +81,6 @@ void CallgrindParserTests::cleanup()
 {
 }
 
-ParseDataPtr parseDataFile(const QString &dataFile)
-{
-    Parser p;
-    p.parse(Utils::FilePath::fromString(dataFile));
-    return p.parserData();
-}
-
 void CallgrindParserTests::testHeaderData()
 {
     const ParseDataPtr data(parseDataFile(dataFile("simpleFunction.out")));