From 06f305265b5f8a1e90d5369312298fe14a3f34ba Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Mon, 15 Feb 2021 17:11:11 +0100 Subject: [PATCH] ClassView: Minimize the mutating API of ParserTreeItem Make the API of ParserTreeItem const only. Reorganize the code a bit: Transform ParserTreeItem::add() method into private mergeWith() and add a static method ParserTreeItem::mergeTrees(). Remove ParserTreeItem::copy(), as in case of adding projects we may use directly the instance from cache. Remove now unneeded Parser::addProject() and use directly getCachedOrParseProjectTree(). Transform ParserTreeItem::copyTree() into private cloneTree() and make it a const method. Move document symbols parsing code from Parser into ParserTreeItem and add a static method ParserTreeItem::parseDocument(). Remove ParserTreeItem::addChild() and provide instead an overloaded constructor. Allocate QElapedTimer object only when debug is on. Fix some const correctness. Remove some ugly usings. Added some TODOs. Task-number: QTCREATORBUG-25317 Change-Id: I6e7c48bb118b0d826fbb463cae94d59bf43a6938 Reviewed-by: Christian Kandeler --- src/plugins/classview/classviewparser.cpp | 176 +++-------- src/plugins/classview/classviewparser.h | 7 +- .../classview/classviewparsertreeitem.cpp | 284 ++++++++++-------- .../classview/classviewparsertreeitem.h | 32 +- 4 files changed, 208 insertions(+), 291 deletions(-) diff --git a/src/plugins/classview/classviewparser.cpp b/src/plugins/classview/classviewparser.cpp index 2dea77edc97..27db704c128 100644 --- a/src/plugins/classview/classviewparser.cpp +++ b/src/plugins/classview/classviewparser.cpp @@ -29,14 +29,9 @@ // cplusplus shared library. the same folder (cplusplus) #include -#include -#include -#include // other #include -#include -#include #include #include #include @@ -93,8 +88,6 @@ public: //! Get document from documentList CPlusPlus::Document::Ptr document(const QString &fileName) const; - CPlusPlus::Overview overview; - QTimer timer; struct DocumentCache { @@ -253,119 +246,38 @@ ParserTreeItem::ConstPtr Parser::findItemByRoot(const QStandardItem *item, bool ParserTreeItem::ConstPtr Parser::parse() { - QElapsedTimer time; - if (debug) - time.start(); - - ParserTreeItem::Ptr rootItem(new ParserTreeItem()); - - // check all projects - for (const Project *prj : SessionManager::projects()) { - ParserTreeItem::Ptr item; - QString prjName(prj->displayName()); - QString prjType = prj->projectFilePath().toString(); - SymbolInformation inf(prjName, prjType); - item = ParserTreeItem::Ptr(new ParserTreeItem()); - - addFlatTree(item, prj); - - item->setIcon(prj->containerNode()->icon()); - - rootItem->appendChild(item, inf); + QScopedPointer timer; + if (debug) { + timer.reset(new QElapsedTimer()); + timer->start(); } - if (debug) + QHash projectTrees; + + // TODO: move a call to SessionManager::projects() out of this thread + for (const Project *prj : SessionManager::projects()) { + const QString prjName(prj->displayName()); + const QString prjType = prj->projectFilePath().toString(); + const SymbolInformation inf(prjName, prjType); + + ParserTreeItem::Ptr item = addFlatTree(prj); + if (item.isNull()) + continue; + // TODO: remove a call to icon + item->setIcon(prj->containerNode()->icon()); + projectTrees.insert(inf, item); + } + + ParserTreeItem::Ptr rootItem(new ParserTreeItem(projectTrees)); + + if (debug) { qDebug() << "Class View:" << QDateTime::currentDateTime().toString() - << "Parsed in " << time.elapsed() << "msecs."; + << "Parsed in " << timer->elapsed() << "msecs."; + } return rootItem; } -/*! - Parses the project with the \a projectId and adds the documents - from the \a fileList to the tree item \a item. -*/ - -void Parser::addProject(const ParserTreeItem::Ptr &item, const QStringList &fileList, - const QString &projectId) -{ - // recalculate cache tree if needed - ParserTreeItem::Ptr prj(getCachedOrParseProjectTree(fileList, projectId)); - if (item.isNull()) - return; - - // if there is an item - copy project tree to that item - item->copy(prj); -} - -/*! - Parses \a symbol and adds the results to \a item (as a parent). -*/ - -void Parser::addSymbol(const ParserTreeItem::Ptr &item, const CPlusPlus::Symbol *symbol) -{ - if (item.isNull() || !symbol) - return; - - // easy solution - lets add any scoped symbol and - // any symbol which does not contain :: in the name - - //! \todo collect statistics and reorder to optimize - if (symbol->isForwardClassDeclaration() - || symbol->isExtern() - || symbol->isFriend() - || symbol->isGenerated() - || symbol->isUsingNamespaceDirective() - || symbol->isUsingDeclaration() - ) - return; - - const CPlusPlus::Name *symbolName = symbol->name(); - if (symbolName && symbolName->isQualifiedNameId()) - return; - - QString name = d->overview.prettyName(symbolName).trimmed(); - QString type = d->overview.prettyType(symbol->type()).trimmed(); - int iconType = CPlusPlus::Icons::iconTypeForSymbol(symbol); - - SymbolInformation information(name, type, iconType); - - ParserTreeItem::Ptr itemAdd; - - // If next line will be removed, 5% speed up for the initial parsing. - // But there might be a problem for some files ??? - // Better to improve qHash timing - itemAdd = item->child(information); - - if (itemAdd.isNull()) - itemAdd = ParserTreeItem::Ptr(new ParserTreeItem()); - - // locations have 1-based column in Symbol, use the same here. - SymbolLocation location(QString::fromUtf8(symbol->fileName() , symbol->fileNameLength()), - symbol->line(), symbol->column()); - itemAdd->addSymbolLocation(location); - - // prevent showing a content of the functions - if (!symbol->isFunction()) { - if (const CPlusPlus::Scope *scope = symbol->asScope()) { - CPlusPlus::Scope::iterator cur = scope->memberBegin(); - CPlusPlus::Scope::iterator last = scope->memberEnd(); - while (cur != last) { - const CPlusPlus::Symbol *curSymbol = *cur; - ++cur; - if (!curSymbol) - continue; - - addSymbol(itemAdd, curSymbol); - } - } - } - - // if item is empty and has not to be added - if (!(symbol->isNamespace() && itemAdd->childCount() == 0)) - item->appendChild(itemAdd, information); -} - /*! Parses the project with the \a projectId and adds the documents from the \a fileList to the project. Updates the internal cached tree for this @@ -377,23 +289,24 @@ ParserTreeItem::Ptr Parser::getParseProjectTree(const QStringList &fileList, { //! \todo Way to optimize - for documentUpdate - use old cached project and subtract //! changed files only (old edition), and add curent editions - ParserTreeItem::Ptr item(new ParserTreeItem()); + + QList docTrees; unsigned revision = 0; - foreach (const QString &file, fileList) { + for (const QString &file : fileList) { const CPlusPlus::Document::Ptr &doc = d->document(file); if (doc.isNull()) continue; revision += doc->revision(); - ParserTreeItem::ConstPtr list = getCachedOrParseDocumentTree(doc); - if (list.isNull()) + const ParserTreeItem::ConstPtr docTree = getCachedOrParseDocumentTree(doc); + if (docTree.isNull()) continue; - - // add list to out document - item->add(list); + docTrees.append(docTree); } + ParserTreeItem::Ptr item = ParserTreeItem::mergeTrees(docTrees); + // update the cache if (!projectId.isEmpty()) { ParserPrivate::ProjectCache &projectCache = d->m_projectCache[projectId]; @@ -410,7 +323,7 @@ ParserTreeItem::Ptr Parser::getParseProjectTree(const QStringList &fileList, */ ParserTreeItem::Ptr Parser::getCachedOrParseProjectTree(const QStringList &fileList, - const QString &projectId) + const QString &projectId) { const auto it = d->m_projectCache.constFind(projectId); if (it != d->m_projectCache.constEnd() && !it.value().tree.isNull()) { @@ -447,11 +360,7 @@ ParserTreeItem::ConstPtr Parser::getParseDocumentTree(const CPlusPlus::Document: if (!d->fileList.contains(fileName)) return ParserTreeItem::ConstPtr(); - ParserTreeItem::Ptr itemPtr(new ParserTreeItem()); - - const unsigned total = doc->globalSymbolCount(); - for (unsigned i = 0; i < total; ++i) - addSymbol(itemPtr, doc->globalSymbolAt(i)); + ParserTreeItem::Ptr itemPtr = ParserTreeItem::parseDocument(doc); d->m_documentCache.insert(fileName, { doc->revision(), itemPtr, doc } ); return itemPtr; @@ -542,7 +451,7 @@ void Parser::resetData(const CPlusPlus::Snapshot &snapshot) // recalculate file list FilePaths fileList; - // check all projects + // TODO: move a call to SessionManager::projects() out of this thread for (const Project *prj : SessionManager::projects()) fileList += prj->files(Project::SourceFiles); setFileList(Utils::transform(fileList, &FilePath::toString)); @@ -582,6 +491,7 @@ void Parser::requestCurrentState() emit treeDataUpdate(std); } +// TODO: don't use Project class in this thread QStringList Parser::getAllFiles(const Project *project) { if (!project) @@ -598,15 +508,17 @@ QStringList Parser::getAllFiles(const Project *project) return fileList; } -void Parser::addFlatTree(const ParserTreeItem::Ptr &item, const Project *project) +// TODO: don't use Project class in this thread +ParserTreeItem::Ptr Parser::addFlatTree(const Project *project) { if (!project) - return; + return {}; - QStringList fileList = getAllFiles(project); + const QStringList fileList = getAllFiles(project); + if (fileList.isEmpty()) + return {}; - if (fileList.count() > 0) - addProject(item, fileList, project->projectFilePath().toString()); + return getCachedOrParseProjectTree(fileList, project->projectFilePath().toString()); } } // namespace Internal diff --git a/src/plugins/classview/classviewparser.h b/src/plugins/classview/classviewparser.h index 7de47cfc806..65299e87b13 100644 --- a/src/plugins/classview/classviewparser.h +++ b/src/plugins/classview/classviewparser.h @@ -29,7 +29,6 @@ #include "classviewparsertreeitem.h" -#include #include // might be changed to forward declaration - is not done to be less dependent @@ -70,10 +69,6 @@ signals: private: void setFileList(const QStringList &fileList); void resetData(const CPlusPlus::Snapshot &snapshot); - void addProject(const ParserTreeItem::Ptr &item, const QStringList &fileList, - const QString &projectId = QString()); - - void addSymbol(const ParserTreeItem::Ptr &item, const CPlusPlus::Symbol *symbol); ParserTreeItem::ConstPtr getParseDocumentTree(const CPlusPlus::Document::Ptr &doc); ParserTreeItem::ConstPtr getCachedOrParseDocumentTree(const CPlusPlus::Document::Ptr &doc); @@ -84,7 +79,7 @@ private: ParserTreeItem::ConstPtr findItemByRoot(const QStandardItem *item, bool skipRoot = false) const; QStringList getAllFiles(const ProjectExplorer::Project *project); - void addFlatTree(const ParserTreeItem::Ptr &item, const ProjectExplorer::Project *project); + ParserTreeItem::Ptr addFlatTree(const ProjectExplorer::Project *project); //! Private class data pointer ParserPrivate *d; diff --git a/src/plugins/classview/classviewparsertreeitem.cpp b/src/plugins/classview/classviewparsertreeitem.cpp index 65b1e2214c0..ca8dc8a4e55 100644 --- a/src/plugins/classview/classviewparsertreeitem.cpp +++ b/src/plugins/classview/classviewparsertreeitem.cpp @@ -29,6 +29,11 @@ #include "classviewconstants.h" #include "classviewutils.h" +#include +#include +#include +#include +#include #include #include @@ -42,6 +47,8 @@ namespace ClassView { namespace Internal { +static CPlusPlus::Overview g_overview; + ///////////////////////////////// ParserTreeItemPrivate ////////////////////////////////// /*! @@ -53,16 +60,121 @@ namespace Internal { class ParserTreeItemPrivate { public: - //! symbol locations - QSet symbolLocations; + void mergeWith(const ParserTreeItem::ConstPtr &target); + void mergeSymbol(const CPlusPlus::Symbol *symbol); + ParserTreeItem::Ptr cloneTree() const; - //! symbol information QHash symbolInformations; - - //! An icon + QSet symbolLocations; QIcon icon; }; +void ParserTreeItemPrivate::mergeWith(const ParserTreeItem::ConstPtr &target) +{ + if (target.isNull()) + return; + + symbolLocations.unite(target->d->symbolLocations); + + // merge children + for (auto it = target->d->symbolInformations.cbegin(); + it != target->d->symbolInformations.cend(); ++it) { + const SymbolInformation &inf = it.key(); + const ParserTreeItem::Ptr &targetChild = it.value(); + + ParserTreeItem::Ptr child = symbolInformations.value(inf); + if (!child.isNull()) { + child->d->mergeWith(targetChild); + } else { + const ParserTreeItem::Ptr clone = targetChild.isNull() ? ParserTreeItem::Ptr() + : targetChild->d->cloneTree(); + symbolInformations.insert(inf, clone); + } + } +} + +void ParserTreeItemPrivate::mergeSymbol(const CPlusPlus::Symbol *symbol) +{ + if (!symbol) + return; + + // easy solution - lets add any scoped symbol and + // any symbol which does not contain :: in the name + + //! \todo collect statistics and reorder to optimize + if (symbol->isForwardClassDeclaration() + || symbol->isExtern() + || symbol->isFriend() + || symbol->isGenerated() + || symbol->isUsingNamespaceDirective() + || symbol->isUsingDeclaration() + ) + return; + + const CPlusPlus::Name *symbolName = symbol->name(); + if (symbolName && symbolName->isQualifiedNameId()) + return; + + QString name = g_overview.prettyName(symbolName).trimmed(); + QString type = g_overview.prettyType(symbol->type()).trimmed(); + int iconType = CPlusPlus::Icons::iconTypeForSymbol(symbol); + + SymbolInformation information(name, type, iconType); + + // If next line will be removed, 5% speed up for the initial parsing. + // But there might be a problem for some files ??? + // Better to improve qHash timing + ParserTreeItem::Ptr childItem = symbolInformations.value(information); + + if (childItem.isNull()) + childItem = ParserTreeItem::Ptr(new ParserTreeItem()); + + // locations have 1-based column in Symbol, use the same here. + SymbolLocation location(QString::fromUtf8(symbol->fileName() , symbol->fileNameLength()), + symbol->line(), symbol->column()); + + childItem->d->symbolLocations.insert(location); + + // prevent showing a content of the functions + if (!symbol->isFunction()) { + if (const CPlusPlus::Scope *scope = symbol->asScope()) { + CPlusPlus::Scope::iterator cur = scope->memberBegin(); + CPlusPlus::Scope::iterator last = scope->memberEnd(); + while (cur != last) { + const CPlusPlus::Symbol *curSymbol = *cur; + ++cur; + if (!curSymbol) + continue; + + childItem->d->mergeSymbol(curSymbol); + } + } + } + + // if item is empty and has not to be added + if (!symbol->isNamespace() || childItem->childCount()) + symbolInformations.insert(information, childItem); +} + +/*! + Creates a deep clone of this tree. +*/ +ParserTreeItem::Ptr ParserTreeItemPrivate::cloneTree() const +{ + ParserTreeItem::Ptr newItem(new ParserTreeItem); + newItem->d->symbolLocations = symbolLocations; + newItem->d->icon = icon; + + for (auto it = symbolInformations.cbegin(); it != symbolInformations.cend(); ++it) { + ParserTreeItem::ConstPtr child = it.value(); + if (child.isNull()) + continue; + newItem->d->symbolInformations.insert(it.key(), child->d->cloneTree()); + } + + return newItem; +} + ///////////////////////////////// ParserTreeItem ////////////////////////////////// /*! @@ -72,72 +184,22 @@ public: Not virtual - to speed up its work. */ -ParserTreeItem::ParserTreeItem() : - d(new ParserTreeItemPrivate()) +ParserTreeItem::ParserTreeItem() + : d(new ParserTreeItemPrivate()) { } +ParserTreeItem::ParserTreeItem(const QHash &children) + : d(new ParserTreeItemPrivate({children, {}, {}})) +{ +} + + ParserTreeItem::~ParserTreeItem() { delete d; } -/*! - Copies a parser tree item from the location specified by \a from to this - item. -*/ - -void ParserTreeItem::copy(const ParserTreeItem::ConstPtr &from) -{ - if (from.isNull()) - return; - - d->symbolLocations = from->d->symbolLocations; - d->icon = from->d->icon; - d->symbolInformations = from->d->symbolInformations; -} - -/*! - \fn void copyTree(const ParserTreeItem::ConstPtr &from) - Copies a parser tree item with children from the location specified by - \a from to this item. -*/ - -void ParserTreeItem::copyTree(const ParserTreeItem::ConstPtr &target) -{ - if (target.isNull()) - return; - - // copy content - d->symbolLocations = target->d->symbolLocations; - d->icon = target->d->icon; - d->symbolInformations.clear(); - - // reserve memory -// int amount = qMin(100 , target->d_ptr->symbolInformations.count() * 2); -// d_ptr->symbolInformations.reserve(amount); - - // every child - CitSymbolInformations cur = target->d->symbolInformations.constBegin(); - CitSymbolInformations end = target->d->symbolInformations.constEnd(); - - for (; cur != end; ++cur) { - ParserTreeItem::Ptr item(new ParserTreeItem()); - item->copyTree(cur.value()); - appendChild(item, cur.key()); - } -} - -/*! - Adds information about symbol location from a \location. - \sa SymbolLocation, removeSymbolLocation, symbolLocations -*/ - -void ParserTreeItem::addSymbolLocation(const SymbolLocation &location) -{ - d->symbolLocations.insert(location); -} - /*! Gets information about symbol positions. \sa SymbolLocation, addSymbolLocation, removeSymbolLocation @@ -148,19 +210,6 @@ QSet ParserTreeItem::symbolLocations() const return d->symbolLocations; } -/*! - Appends the child item \a item to \a inf symbol information. -*/ - -void ParserTreeItem::appendChild(const ParserTreeItem::Ptr &item, const SymbolInformation &inf) -{ - // removeChild must be used to remove an item - if (item.isNull()) - return; - - d->symbolInformations[inf] = item; -} - /*! Returns the child item specified by \a inf symbol information. */ @@ -197,38 +246,24 @@ void ParserTreeItem::setIcon(const QIcon &icon) d->icon = icon; } -/*! - Adds an internal state with \a target, which contains the correct current - state. -*/ - -void ParserTreeItem::add(const ParserTreeItem::ConstPtr &target) +ParserTreeItem::Ptr ParserTreeItem::parseDocument(const CPlusPlus::Document::Ptr &doc) { - if (target.isNull()) - return; + Ptr item(new ParserTreeItem()); - // add locations - d->symbolLocations = d->symbolLocations.unite(target->d->symbolLocations); + const unsigned total = doc->globalSymbolCount(); + for (unsigned i = 0; i < total; ++i) + item->d->mergeSymbol(doc->globalSymbolAt(i)); - // add children - // every target child - CitSymbolInformations cur = target->d->symbolInformations.constBegin(); - CitSymbolInformations end = target->d->symbolInformations.constEnd(); - while (cur != end) { - const SymbolInformation &inf = cur.key(); - const ParserTreeItem::Ptr &targetChild = cur.value(); + return item; +} - ParserTreeItem::Ptr child = d->symbolInformations.value(inf); - if (!child.isNull()) { - child->add(targetChild); - } else { - ParserTreeItem::Ptr add(new ParserTreeItem()); - add->copyTree(targetChild); - d->symbolInformations[inf] = add; - } - // next item - ++cur; - } +ParserTreeItem::Ptr ParserTreeItem::mergeTrees(const QList &docTrees) +{ + Ptr item(new ParserTreeItem()); + for (const ConstPtr &docTree : docTrees) + item->d->mergeWith(docTree); + + return item; } /*! @@ -256,23 +291,14 @@ void ParserTreeItem::convertTo(QStandardItem *item) const if (!item) return; - QMap map; - // convert to map - to sort it - CitSymbolInformations curHash = d->symbolInformations.constBegin(); - CitSymbolInformations endHash = d->symbolInformations.constEnd(); - while (curHash != endHash) { - map.insert(curHash.key(), curHash.value()); - ++curHash; - } + QMap map; + for (auto it = d->symbolInformations.cbegin(); it != d->symbolInformations.cend(); ++it) + map.insert(it.key(), it.value()); - using MapCitSymbolInformations = QMap::const_iterator; - // add to item - MapCitSymbolInformations cur = map.constBegin(); - MapCitSymbolInformations end = map.constEnd(); - while (cur != end) { - const SymbolInformation &inf = cur.key(); - ParserTreeItem::Ptr ptr = cur.value(); + for (auto it = map.cbegin(); it != map.cend(); ++it) { + const SymbolInformation &inf = it.key(); + Ptr ptr = it.value(); auto add = new QStandardItem; add->setData(inf.name(), Constants::SymbolNameRole); @@ -291,7 +317,6 @@ void ParserTreeItem::convertTo(QStandardItem *item) const add->setData(locationsToRole(ptr->symbolLocations()), Constants::SymbolLocationsRole); } item->appendRow(add); - ++cur; } } @@ -325,18 +350,15 @@ void ParserTreeItem::fetchMore(QStandardItem *item) const Debug dump. */ -void ParserTreeItem::debugDump(int ident) const +void ParserTreeItem::debugDump(int indent) const { - CitSymbolInformations curHash = d->symbolInformations.constBegin(); - CitSymbolInformations endHash = d->symbolInformations.constEnd(); - while (curHash != endHash) { - const SymbolInformation &inf = curHash.key(); - qDebug() << QString(2*ident, QLatin1Char(' ')) << inf.iconType() << inf.name() << inf.type() - << curHash.value().isNull(); - if (!curHash.value().isNull()) - curHash.value()->debugDump(ident + 1); - - ++curHash; + for (auto it = d->symbolInformations.cbegin(); it != d->symbolInformations.cend(); ++it) { + const SymbolInformation &inf = it.key(); + const Ptr &child = it.value(); + qDebug() << QString(2 * indent, QLatin1Char(' ')) << inf.iconType() << inf.name() + << inf.type() << child.isNull(); + if (!child.isNull()) + child->debugDump(indent + 1); } } diff --git a/src/plugins/classview/classviewparsertreeitem.h b/src/plugins/classview/classviewparsertreeitem.h index 028e81b2597..f34499a0e40 100644 --- a/src/plugins/classview/classviewparsertreeitem.h +++ b/src/plugins/classview/classviewparsertreeitem.h @@ -28,6 +28,8 @@ #include "classviewsymbollocation.h" #include "classviewsymbolinformation.h" +#include + #include #include @@ -46,42 +48,28 @@ public: public: ParserTreeItem(); + ParserTreeItem(const QHash &children); ~ParserTreeItem(); - void copyTree(const ParserTreeItem::ConstPtr &from); - - void copy(const ParserTreeItem::ConstPtr &from); - - void addSymbolLocation(const SymbolLocation &location); + static Ptr parseDocument(const CPlusPlus::Document::Ptr &doc); + static Ptr mergeTrees(const QList &docTrees); QSet symbolLocations() const; - - void appendChild(const ParserTreeItem::Ptr &item, const SymbolInformation &inf); - - ParserTreeItem::Ptr child(const SymbolInformation &inf) const; - + Ptr child(const SymbolInformation &inf) const; int childCount() const; - void convertTo(QStandardItem *item) const; - - // additional properties - //! Assigned icon + // TODO: Remove icon from this API, we can't use QIcons in non-GUI thread QIcon icon() const; - - //! Set an icon for this tree node void setIcon(const QIcon &icon); - void add(const ParserTreeItem::ConstPtr &target); - + void convertTo(QStandardItem *item) const; bool canFetchMore(QStandardItem *item) const; - void fetchMore(QStandardItem *item) const; - void debugDump(int ident = 0) const; + void debugDump(int indent = 0) const; private: - using CitSymbolInformations = QHash::const_iterator; - //! Private class data pointer + friend class ParserTreeItemPrivate; ParserTreeItemPrivate *d; };