Clang: Don't index already indexed symbols

We don't index system headers any more and introduce a first step to
decrease double indexing. For that we introduces the SourcesManager which
so far tells you only if a file was already indexed for a certain time
stamp.

Change-Id: Icde54465693ca84a622764c595635cac365c0111
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>

src/tools/clangrefactoringbackend/source/collectmacrospreprocessorcallbacks.h

@@ -30,6 +30,7 @@
 #include "sourcedependency.h"
 #include "sourcelocationsutils.h"
 #include "sourcelocationentry.h"
+#include "sourcesmanager.h"
 #include "symbolentry.h"
 #include "usedmacro.h"
 
@@ -54,8 +55,9 @@ public:
                                        SourceDependencies &sourceDependencies,
                                        FilePathCachingInterface &filePathCache,
                                        const clang::SourceManager &sourceManager,
-                                       std::shared_ptr<clang::Preprocessor> &&preprocessor)
+                                       std::shared_ptr<clang::Preprocessor> &&preprocessor,
-        : SymbolsVisitorBase(filePathCache, &sourceManager),
+                                       SourcesManager &sourcesManager)
+        : SymbolsVisitorBase(filePathCache, &sourceManager, sourcesManager),
           m_preprocessor(std::move(preprocessor)),
           m_sourceDependencies(sourceDependencies),
           m_symbolEntries(symbolEntries),
@@ -165,6 +167,7 @@ public:
     {
         filterOutHeaderGuards();
         mergeUsedMacros();
+        m_sourcesManager.updateModifiedTimeStamps();
     }
 
     void filterOutHeaderGuards()
@@ -205,6 +208,9 @@ public:
     void addUsedMacro(const clang::Token &macroNameToken,
                        const clang::MacroDefinition &macroDefinition)
     {
+        if (isInSystemHeader(macroNameToken.getLocation()))
+            return;
+
         clang::MacroInfo *macroInfo = macroDefinition.getMacroInfo();
         UsedMacro usedMacro{macroNameToken.getIdentifierInfo()->getName(),
                               filePathId(macroNameToken.getLocation())};
@@ -229,12 +235,17 @@ public:
         return nullptr;
     }
 
+    bool isInSystemHeader(clang::SourceLocation sourceLocation) const
+    {
+        return m_sourceManager->isInSystemHeader(sourceLocation);
+    }
+
     void addMacroAsSymbol(const clang::Token &macroNameToken,
                           const clang::MacroInfo *macroInfo,
                           SourceLocationKind symbolType)
     {
         clang::SourceLocation sourceLocation = macroNameToken.getLocation();
-        if (macroInfo && sourceLocation.isFileID()) {
+        if (macroInfo && sourceLocation.isFileID() && !isInSystemHeader(sourceLocation)) {
             FilePathId fileId = filePathId(sourceLocation);
             if (fileId.isValid()) {
                 auto macroName = macroNameToken.getIdentifierInfo()->getName();

src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.cpp

@@ -42,7 +42,8 @@ bool CollectMacrosSourceFileCallbacks::handleBeginSource(clang::CompilerInstance
                 m_sourceDependencies,
                 m_filePathCache,
                 compilerInstance.getSourceManager(),
-                compilerInstance.getPreprocessorPtr());
+                compilerInstance.getPreprocessorPtr(),
+                m_sourcesManager);
 
     compilerInstance.getPreprocessorPtr()->addPPCallbacks(std::move(callbacks));
 

src/tools/clangrefactoringbackend/source/collectmacrossourcefilecallbacks.h

@@ -37,15 +37,19 @@
 
 namespace ClangBackEnd {
 
+class SourcesManager;
+
 class CollectMacrosSourceFileCallbacks : public clang::tooling::SourceFileCallbacks
 {
 public:
     CollectMacrosSourceFileCallbacks(SymbolEntries &symbolEntries,
                                      SourceLocationEntries &sourceLocationEntries,
-                                     FilePathCachingInterface &filePathCache)
+                                     FilePathCachingInterface &filePathCache,
+                                     SourcesManager &sourcesManager)
         : m_symbolEntries(symbolEntries),
           m_sourceLocationEntries(sourceLocationEntries),
-          m_filePathCache(filePathCache)
+          m_filePathCache(filePathCache),
+          m_sourcesManager(sourcesManager)
     {
     }
 
@@ -91,6 +95,7 @@ private:
     SymbolEntries &m_symbolEntries;
     SourceLocationEntries &m_sourceLocationEntries;
     FilePathCachingInterface &m_filePathCache;
+    SourcesManager &m_sourcesManager;
 };
 
 } // namespace ClangBackEnd

src/tools/clangrefactoringbackend/source/indexdataconsumer.cpp

@@ -98,6 +98,12 @@ SymbolKindAndTags symbolKindAndTags(const clang::Decl *declaration)
     static IndexingDeclVisitor visitor;
     return visitor.Visit(declaration);
 }
+
+bool isContextIndependentDeclaration(const clang::Decl *declaration)
+{
+    return clang::dyn_cast<clang::ValueDecl>(declaration)
+            || clang::dyn_cast<clang::TypeDecl>(declaration);
+}
 }
 
 bool IndexDataConsumer::handleDeclOccurence(const clang::Decl *declaration,
@@ -107,12 +113,14 @@ bool IndexDataConsumer::handleDeclOccurence(const clang::Decl *declaration,
                                             unsigned offset,
                                             IndexDataConsumer::ASTNodeInfo astNodeInfo)
 {
-
     const auto *namedDeclaration = clang::dyn_cast<clang::NamedDecl>(declaration);
     if (namedDeclaration) {
         if (!namedDeclaration->getIdentifier())
             return true;
 
+        if (alreadyParsed(fileId) && isContextIndependentDeclaration(declaration))
+            return true;
+
         SymbolIndex globalId = toSymbolIndex(declaration->getCanonicalDecl());
         clang::SourceLocation sourceLocation = m_sourceManager->getLocForStartOfFile(fileId).getLocWithOffset(offset);
 

src/tools/clangrefactoringbackend/source/indexdataconsumer.h

@@ -42,8 +42,9 @@ class IndexDataConsumer : public clang::index::IndexDataConsumer,
 public:
     IndexDataConsumer(SymbolEntries &symbolEntries,
                       SourceLocationEntries &sourceLocationEntries,
-                      FilePathCachingInterface &filePathCache)
+                      FilePathCachingInterface &filePathCache,
-        : SymbolsVisitorBase(filePathCache, nullptr),
+                      SourcesManager &sourcesManager)
+        : SymbolsVisitorBase(filePathCache, nullptr, sourcesManager),
           m_symbolEntries(symbolEntries),
           m_sourceLocationEntries(sourceLocationEntries)
     {}

src/tools/clangrefactoringbackend/source/symbolscollector.cpp

@@ -30,9 +30,9 @@
 namespace ClangBackEnd {
 
 SymbolsCollector::SymbolsCollector(FilePathCachingInterface &filePathCache)
-    : m_indexDataConsumer(std::make_shared<IndexDataConsumer>(m_symbolEntries, m_sourceLocationEntries, filePathCache)),
+    : m_indexDataConsumer(std::make_shared<IndexDataConsumer>(m_symbolEntries, m_sourceLocationEntries, filePathCache, m_sourcesManager)),
       m_collectSymbolsAction(m_indexDataConsumer),
-      m_collectMacrosSourceFileCallbacks(m_symbolEntries, m_sourceLocationEntries, filePathCache),
+      m_collectMacrosSourceFileCallbacks(m_symbolEntries, m_sourceLocationEntries, filePathCache, m_sourcesManager),
       m_filePathCache(filePathCache)
 {
 }

src/tools/clangrefactoringbackend/source/symbolscollector.h

@@ -28,6 +28,7 @@
 #include "clangtool.h"
 #include "collectmacrossourcefilecallbacks.h"
 #include "collectsymbolsaction.h"
+#include "sourcesmanager.h"
 #include "symbolscollectorinterface.h"
 
 #include <filepathcachingfwd.h>
@@ -62,6 +63,7 @@ private:
     std::shared_ptr<IndexDataConsumer> m_indexDataConsumer;
     CollectSymbolsAction m_collectSymbolsAction;
     CollectMacrosSourceFileCallbacks m_collectMacrosSourceFileCallbacks;
+    SourcesManager m_sourcesManager;
     FilePathCachingInterface &m_filePathCache;
 };
 

src/tools/clangrefactoringbackend/source/symbolsvisitorbase.h

@@ -26,6 +26,8 @@
 #pragma once
 
 #include "sourcelocationsutils.h"
+#include "sourcesmanager.h"
+#include "symbolentry.h"
 
 #include <filepathcachinginterface.h>
 
@@ -43,9 +45,11 @@ class SymbolsVisitorBase
 {
 public:
     SymbolsVisitorBase(FilePathCachingInterface &filePathCache,
-                       const clang::SourceManager *sourceManager)
+                       const clang::SourceManager *sourceManager,
+                       SourcesManager &sourcesManager)
         : m_filePathCache(filePathCache),
-          m_sourceManager(sourceManager)
+          m_sourceManager(sourceManager),
+          m_sourcesManager(sourcesManager)
     {}
 
     FilePathId filePathId(clang::SourceLocation sourceLocation)
@@ -56,6 +60,19 @@ public:
         return filePathId(fileEntry);
     }
 
+    bool alreadyParsed(clang::FileID fileId)
+    {
+        const clang::FileEntry *fileEntry = m_sourceManager->getFileEntryForID(fileId);
+
+        return m_sourcesManager.alreadyParsed(filePathId(fileEntry),
+                                              fileEntry->getModificationTime());
+    }
+
+    bool alreadyParsed(clang::SourceLocation sourceLocation)
+    {
+        return alreadyParsed(m_sourceManager->getFileID(sourceLocation));
+    }
+
     FilePathId filePathId(const clang::FileEntry *fileEntry)
     {
         if (fileEntry) {
@@ -128,10 +145,17 @@ public:
         m_sourceManager = sourceManager;
     }
 
+    bool isInSystemHeader(clang::FileID fileId) const
+    {
+        return clang::SrcMgr::isSystem(
+                    m_sourceManager->getSLocEntry(fileId).getFile().getFileCharacteristic());
+    }
+
 protected:
     std::vector<FilePathId> m_filePathIndices;
     FilePathCachingInterface &m_filePathCache;
     const clang::SourceManager *m_sourceManager = nullptr;
+    SourcesManager &m_sourcesManager;
 };
 
 } // namespace ClangBackend

tests/unit/unittest/data/include/symbolscollector_unmodified_header.h

@@ -0,0 +1,11 @@
+#pragma once
+
+#define HEADER_DEFINE
+
+void HeaderFunction();
+
+class Class
+{
+public:
+    int Member = 20;
+};

tests/unit/unittest/data/symbolscollector_unmodified.cpp

@@ -0,0 +1,5 @@
+#include <symbolscollector_unmodified_header.h>
+
+#define MAINFILE_DEFINE
+
+void MainFileFunction();

tests/unit/unittest/data/symbolscollector_unmodified2.cpp

@@ -0,0 +1,5 @@
+#include <symbolscollector_unmodified_header.h>
+
+#define MAINFILE_DEFINE
+
+void MainFileFunction();

tests/unit/unittest/symbolscollector-test.cpp

@@ -707,4 +707,34 @@ TEST_F(SymbolsCollector, IsVariableSymbol)
                         HasSymbolKind(SymbolKind::Variable))));
 }
 
+
+TEST_F(SymbolsCollector, DontIndexUnmodifiedHeaderFiles)
+{
+    collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified.cpp")}, {"cc", "-I", {TESTDATA_DIR, "/include"}});
+    collector.collectSymbols();
+    collector.clear();
+    collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified2.cpp")}, {"cc", "-I", {TESTDATA_DIR, "/include"}});
+
+    collector.collectSymbols();
+
+    ASSERT_THAT(collector.symbols(),
+                AllOf(
+                    Not(Contains(HasSymbolName("HeaderFunction"))),
+                    Not(Contains(HasSymbolName("Class"))),
+                    Not(Contains(HasSymbolName("Member")))));
+}
+
+TEST_F(SymbolsCollector, DontIndexSystemIncudes)
+{
+    collector.addFiles({filePathId(TESTDATA_DIR "/symbolscollector_unmodified.cpp")}, {"cc", "-isystem", {TESTDATA_DIR, "/include"}});
+
+    collector.collectSymbols();
+
+    ASSERT_THAT(collector.symbols(),
+                AllOf(
+                    Not(Contains(HasSymbolName("HeaderFunction"))),
+                    Not(Contains(HasSymbolName("Class"))),
+                    Not(Contains(HasSymbolName("Member"))),
+                    Not(Contains(HasSymbolName("HEADER_DEFINE")))));
+}
 }