C++: Fix "reference" file in symbol finder

Througout the initial review the singleton was transformed into an
ordinary class, but a error was introduced: The "reference" file was
incorrectly assumed to be the editors file, which is wrong, since
it should be the declaration file.

Change-Id: Iad3e25a690fa8bd07a18184b24b10f8dea965332
Reviewed-by: Roberto Raggi <roberto.raggi@nokia.com>

src/plugins/cppeditor/cppeditor.cpp

@@ -420,7 +420,7 @@ CPPEditorWidget::CPPEditorWidget(QWidget *parent)
     , m_firstRenameChange(false)
     , m_objcEnabled(false)
     , m_commentsSettings(CppTools::CppToolsSettings::instance()->commentsSettings())
-    , m_symbolFinder(new CppTools::SymbolFinder(QString()))
+    , m_symbolFinder(new CppTools::SymbolFinder)
 {
     m_initialized = false;
     qRegisterMetaType<CppEditor::Internal::SemanticInfo>("CppEditor::Internal::SemanticInfo");
@@ -864,9 +864,7 @@ void CPPEditorWidget::onContentsChanged(int position, int charsRemoved, int char
 }
 
 void CPPEditorWidget::updateFileName()
-{
-    m_symbolFinder.reset(new CppTools::SymbolFinder(file()->fileName()));
-}
+{}
 
 void CPPEditorWidget::jumpToOutlineElement(int)
 {
@@ -2258,9 +2256,9 @@ void CPPEditorWidget::applyDeclDefLinkChanges(bool jumpToMatch)
     updateFunctionDeclDefLink();
 }
 
-CppTools::SymbolFinder *CPPEditorWidget::symbolFinder() const
+QSharedPointer<CppTools::SymbolFinder> CPPEditorWidget::symbolFinder() const
 {
-    return m_symbolFinder.data();
+    return m_symbolFinder;
 }
 
 void CPPEditorWidget::abortDeclDefLink()

src/plugins/cppeditor/cppeditor.h

@@ -50,7 +50,6 @@
 #include <QtCore/QFutureWatcher>
 #include <QtCore/QModelIndex>
 #include <QtCore/QVector>
-#include <QtCore/QScopedPointer>
 
 QT_BEGIN_NAMESPACE
 class QComboBox;
@@ -202,7 +201,7 @@ public:
     QSharedPointer<FunctionDeclDefLink> declDefLink() const;
     void applyDeclDefLinkChanges(bool jumpToMatch);
 
-    CppTools::SymbolFinder *symbolFinder() const;
+    QSharedPointer<CppTools::SymbolFinder> symbolFinder() const;
 
 Q_SIGNALS:
     void outlineModelIndexChanged(const QModelIndex &index);
@@ -337,7 +336,7 @@ private:
     QSharedPointer<FunctionDeclDefLink> m_declDefLink;
 
     CppTools::CommentsSettings m_commentsSettings;
-    QScopedPointer<CppTools::SymbolFinder> m_symbolFinder;
+    QSharedPointer<CppTools::SymbolFinder> m_symbolFinder;
 };
 
 

src/plugins/cppeditor/cppfunctiondecldeflink.cpp

@@ -164,17 +164,15 @@ static QSharedPointer<FunctionDeclDefLink> findLinkHelper(QSharedPointer<Functio
 
     // find the matching decl/def symbol
     Symbol *target = 0;
+    CppTools::SymbolFinder finder;
     if (FunctionDefinitionAST *funcDef = link->sourceDeclaration->asFunctionDefinition()) {
         QList<Declaration *> nameMatch, argumentCountMatch, typeMatch;
-        CppTools::SymbolFinder finder(funcDef->symbol->fileName(), funcDef->symbol->fileNameLength());
         finder.findMatchingDeclaration(LookupContext(link->sourceDocument, snapshot),
                                        funcDef->symbol,
                                        &typeMatch, &argumentCountMatch, &nameMatch);
         if (!typeMatch.isEmpty())
             target = typeMatch.first();
     } else if (link->sourceDeclaration->asSimpleDeclaration()) {
-        CppTools::SymbolFinder finder(link->sourceFunctionDeclarator->symbol->fileName(),
-                                      link->sourceFunctionDeclarator->symbol->fileNameLength());
         target = finder.findMatchingDefinition(link->sourceFunctionDeclarator->symbol, snapshot, true);
     }
     if (!target) {

src/plugins/cppeditor/cppquickfixes.cpp

@@ -1553,7 +1553,7 @@ private:
         {
             Q_ASSERT(fwdClass != 0);
 
-            CppTools::SymbolFinder symbolFinder(fwdClass->fileName(), fwdClass->fileNameLength());
+            CppTools::SymbolFinder symbolFinder;
             if (Class *k =
                     symbolFinder.findMatchingClassDeclaration(fwdClass,
                                                               assistInterface()->snapshot())) {

src/plugins/cpptools/insertionpointlocator.cpp

@@ -517,8 +517,7 @@ static InsertionLocation nextToSurroundingDefinitions(Declaration *declaration,
     }
 
     // find the declaration's definition
-    CppTools::SymbolFinder symbolFinder(surroundingFunctionDecl->fileName(),
-                                        surroundingFunctionDecl->fileNameLength());
+    CppTools::SymbolFinder symbolFinder;
     Symbol *definition = symbolFinder.findMatchingDefinition(surroundingFunctionDecl,
                                                              changes.snapshot());
     if (!definition)
@@ -559,7 +558,7 @@ QList<InsertionLocation> InsertionPointLocator::methodDefinition(
     if (!declaration)
         return result;
 
-    CppTools::SymbolFinder symbolFinder(declaration->fileName(), declaration->fileNameLength());
+    CppTools::SymbolFinder symbolFinder;
     if (Symbol *s = symbolFinder.findMatchingDefinition(declaration,
                                                         m_refactoringChanges.snapshot(),
                                                         true)) {

src/plugins/cpptools/symbolfinder.cpp

@@ -63,13 +63,9 @@ public:
 
 } // end of anonymous namespace
 
+static const int kMaxSize = 2;
 
-SymbolFinder::SymbolFinder(const QString &referenceFileName)
-    : m_referenceFile(referenceFileName)
-{}
-
-SymbolFinder::SymbolFinder(const char *referenceFileName, unsigned referenceFileLength)
-    : m_referenceFile(QString::fromUtf8(referenceFileName, referenceFileLength))
+SymbolFinder::SymbolFinder()
 {}
 
 // strict means the returned symbol has to match exactly,
@@ -82,7 +78,6 @@ Symbol *SymbolFinder::findMatchingDefinition(Symbol *declaration,
         return 0;
 
     QString declFile = QString::fromUtf8(declaration->fileName(), declaration->fileNameLength());
-    Q_ASSERT(declFile == m_referenceFile);
 
     Document::Ptr thisDocument = snapshot.document(declFile);
     if (! thisDocument) {
@@ -97,10 +92,10 @@ Symbol *SymbolFinder::findMatchingDefinition(Symbol *declaration,
         return 0;
     }
 
-    foreach (const QString &fileName, fileIterationOrder(snapshot)) {
+    foreach (const QString &fileName, fileIterationOrder(declFile, snapshot)) {
         Document::Ptr doc = snapshot.document(fileName);
         if (!doc) {
-            clear(fileName);
+            clearCache(declFile, fileName);
             continue;
         }
 
@@ -187,12 +182,11 @@ Class *SymbolFinder::findMatchingClassDeclaration(Symbol *declaration, const Sna
         return 0;
 
     QString declFile = QString::fromUtf8(declaration->fileName(), declaration->fileNameLength());
-    Q_ASSERT(declFile == m_referenceFile);
 
-    foreach (const QString &file, fileIterationOrder(snapshot)) {
+    foreach (const QString &file, fileIterationOrder(declFile, snapshot)) {
         Document::Ptr doc = snapshot.document(file);
         if (!doc) {
-            clear(file);
+            clearCache(declFile, file);
             continue;
         }
 
@@ -289,41 +283,63 @@ QList<Declaration *> SymbolFinder::findMatchingDeclaration(const LookupContext &
     return result;
 }
 
-#include <QtCore/QThread>
-QStringList SymbolFinder::fileIterationOrder(const Snapshot &snapshot)
+QStringList SymbolFinder::fileIterationOrder(const QString &referenceFile, const Snapshot &snapshot)
 {
-    if (m_filePriorityCache.isEmpty()) {
-        foreach (const Document::Ptr &doc, snapshot)
-            insert(doc->fileName());
+    if (m_filePriorityCache.contains(referenceFile)) {
+        checkCacheConsistency(referenceFile, snapshot);
     } else {
-        checkCacheConsistency(snapshot);
+        foreach (Document::Ptr doc, snapshot)
+            insertCache(referenceFile, doc->fileName());
     }
 
-    return m_filePriorityCache.values();
+    trackCacheUse(referenceFile);
+
+    return m_filePriorityCache.value(referenceFile).values();
 }
 
-void SymbolFinder::checkCacheConsistency(const Snapshot &snapshot)
+void SymbolFinder::checkCacheConsistency(const QString &referenceFile, const Snapshot &snapshot)
 {
     // We only check for "new" files, which which are in the snapshot but not in the cache.
     // The counterpart validation for "old" files is done when one tries to access the
     // corresponding document and notices it's now null.
+    const QSet<QString> &meta = m_fileMetaCache.value(referenceFile);
     foreach (const Document::Ptr &doc, snapshot) {
-        if (!m_fileMetaCache.contains(doc->fileName()))
-            insert(doc->fileName());
+        if (!meta.contains(doc->fileName()))
+            insertCache(referenceFile, doc->fileName());
     }
 }
 
-void SymbolFinder::clear(const QString &comparingFile)
+void SymbolFinder::clearCache(const QString &referenceFile, const QString &comparingFile)
 {
-    m_filePriorityCache.remove(computeKey(m_referenceFile, comparingFile), comparingFile);
-    m_fileMetaCache.remove(comparingFile);
+    m_filePriorityCache[referenceFile].remove(computeKey(referenceFile, comparingFile),
+                                              comparingFile);
+    m_fileMetaCache[referenceFile].remove(comparingFile);
 }
 
-void SymbolFinder::insert(const QString &comparingFile)
+void SymbolFinder::insertCache(const QString &referenceFile, const QString &comparingFile)
 {
     // We want an ordering such that the documents with the most common path appear first.
-    m_filePriorityCache.insert(computeKey(m_referenceFile, comparingFile), comparingFile);
-    m_fileMetaCache.insert(comparingFile);
+    m_filePriorityCache[referenceFile].insert(computeKey(referenceFile, comparingFile),
+                                              comparingFile);
+    m_fileMetaCache[referenceFile].insert(comparingFile);
+}
+
+void SymbolFinder::trackCacheUse(const QString &referenceFile)
+{
+    if (!m_recent.isEmpty()) {
+        if (m_recent.last() == referenceFile)
+            return;
+        m_recent.removeOne(referenceFile);
+    }
+
+    m_recent.append(referenceFile);
+
+    // We don't want this to grow too much.
+    if (m_recent.size() > kMaxSize) {
+        const QString &oldest = m_recent.front();
+        m_filePriorityCache.remove(oldest);
+        m_fileMetaCache.remove(oldest);
+    }
 }
 
 int SymbolFinder::computeKey(const QString &referenceFile, const QString &comparingFile)

src/plugins/cpptools/symbolfinder.h

@@ -8,7 +8,6 @@
 
 #include <QtCore/QHash>
 #include <QtCore/QStringList>
-#include <QtCore/QQueue>
 #include <QtCore/QMultiMap>
 #include <QtCore/QSet>
 
@@ -17,8 +16,7 @@ namespace CppTools {
 class CPPTOOLS_EXPORT SymbolFinder
 {
 public:
-    SymbolFinder(const QString &referenceFileName);
-    SymbolFinder(const char *referenceFileName, unsigned referenceFileLength);
+    SymbolFinder();
 
     CPlusPlus::Symbol *findMatchingDefinition(CPlusPlus::Symbol *symbol,
                                               const CPlusPlus::Snapshot &snapshot,
@@ -37,17 +35,20 @@ public:
                                                             CPlusPlus::Function *functionType);
 
 private:
-    QStringList fileIterationOrder(const CPlusPlus::Snapshot &snapshot);
+    QStringList fileIterationOrder(const QString &referenceFile,
+                                   const CPlusPlus::Snapshot &snapshot);
 
-    void checkCacheConsistency(const CPlusPlus::Snapshot &snapshot);
-    void clear(const QString &comparingFile);
-    void insert(const QString &comparingFile);
+    void checkCacheConsistency(const QString &referenceFile, const CPlusPlus::Snapshot &snapshot);
+    void clearCache(const QString &referenceFile, const QString &comparingFile);
+    void insertCache(const QString &referenceFile, const QString &comparingFile);
+
+    void trackCacheUse(const QString &referenceFile);
 
     static int computeKey(const QString &referenceFile, const QString &comparingFile);
 
-    QMultiMap<int, QString> m_filePriorityCache;
-    QSet<QString> m_fileMetaCache;
-    QString m_referenceFile;
+    QHash<QString, QMultiMap<int, QString> > m_filePriorityCache;
+    QHash<QString, QSet<QString> > m_fileMetaCache;
+    QStringList m_recent;
 };
 
 } // namespace CppTools

src/plugins/designer/qtcreatorintegration.cpp

@@ -274,8 +274,7 @@ static Document::Ptr findDefinition(Function *functionDeclaration, int *line)
 {
     if (CppModelManagerInterface *cppModelManager = CppModelManagerInterface::instance()) {
         const Snapshot snapshot = cppModelManager->snapshot();
-        CppTools::SymbolFinder symbolFinder(functionDeclaration->fileName(),
-                                            functionDeclaration->fileNameLength());
+        CppTools::SymbolFinder symbolFinder;
         if (Symbol *def = symbolFinder.findMatchingDefinition(functionDeclaration, snapshot)) {
             if (line)
                 *line = def->line();