Clang: Make file ids unique

Clang file ids are only unique for one query. Because we query in parallel
we have to manage our own unique ids.

Change-Id: I67d57d8b1766cab75ad252a14e57bbf9dc5fdb79
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/libs/clangbackendipc/clangbackendipc-lib.pri

@@ -155,6 +155,7 @@ HEADERS += \
     $$PWD/pchmanagerclientproxy.h \
     $$PWD/projectpartpch.h \
     $$PWD/precompiledheadersupdatedmessage.h \
+    $$PWD/stringcache.h \
     $$PWD/removepchprojectpartsmessage.h \
     $$PWD/clangcodemodelclientmessages.h \
     $$PWD/clangcodemodelservermessages.h \

src/libs/clangbackendipc/filepath.h

@@ -37,66 +37,62 @@ class FilePath
 {
 public:
     FilePath() = default;
-    explicit FilePath(Utils::SmallString &&filePath)
+    explicit FilePath(Utils::PathString &&filePath)
+        : m_path(std::move(filePath))
     {
-        auto foundReverse = std::find(filePath.rbegin(), filePath.rend(), '/');
+        auto foundReverse = std::find(m_path.rbegin(), m_path.rend(), '/');
         auto found = foundReverse.base();
+        --found;
 
-        Utils::SmallString fileName(found, filePath.end());
-        if (foundReverse != filePath.rend())
-            filePath.resize(std::size_t(std::distance(filePath.begin(), --found)));
-
-        directory_ = std::move(filePath);
-        name_ = std::move(fileName);
+        m_slashIndex = std::size_t(std::distance(m_path.begin(), found));
     }
 
-    explicit FilePath(const QString &filePath)
-        : FilePath(Utils::SmallString(filePath))
+    explicit FilePath(const Utils::PathString &filePath)
+        : FilePath(filePath.clone())
     {
     }
 
-    FilePath(Utils::SmallString &&directory, Utils::SmallString &&name)
-        : directory_(std::move(directory)),
-          name_(std::move(name))
-    {}
-
-    const Utils::SmallString &directory() const
+    explicit FilePath(Utils::PathString &&filePath, std::size_t slashIndex)
+        : m_path(std::move(filePath)),
+          m_slashIndex(slashIndex)
     {
-        return directory_;
     }
 
-    Utils::SmallString takeDirectory()
+    explicit FilePath(const QString &filePath)
+        : FilePath(Utils::PathString(filePath))
     {
-        return std::move(directory_);
     }
 
-    const Utils::SmallString &name() const
+    FilePath(const Utils::PathString &directory, const Utils::PathString &name)
+        : m_path({std::move(directory), "/", std::move(name)}),
+          m_slashIndex(directory.size())
+    {}
+
+    Utils::SmallStringView directory() const
     {
-        return name_;
+        return m_path.mid(0, m_slashIndex);
     }
 
-    Utils::SmallString takeName()
+    Utils::SmallStringView name() const
     {
-        return std::move(name_);
+        return m_path.mid(m_slashIndex + 1, m_path.size() - m_slashIndex - 1);
     }
 
-    Utils::PathString path()  const
+    const Utils::PathString &path()  const
     {
-        return {directory_, "/", name_};
+        return m_path;
     }
 
     friend QDataStream &operator<<(QDataStream &out, const FilePath &filePath)
     {
-        out << filePath.directory_;
-        out << filePath.name_;
+        out << filePath.m_path;
 
         return out;
     }
 
     friend QDataStream &operator>>(QDataStream &in, FilePath &filePath)
     {
-        in >> filePath.directory_;
-        in >> filePath.name_;
+        in >> filePath.m_path;
 
         return in;
     }
@@ -110,24 +106,22 @@ public:
 
     friend bool operator==(const FilePath &first, const FilePath &second)
     {
-        return first.name_ == second.name_
-            && first.directory_ == second.directory_;
+        return first.m_path == second.m_path;
     }
 
     friend bool operator<(const FilePath &first, const FilePath &second)
     {
-        return std::tie(first.name_, first.directory_)
-             < std::tie(second.name_, second.directory_);
+        return first.m_path < second.m_path;
     }
 
     FilePath clone() const
     {
-        return FilePath(directory_.clone(), name_.clone());
+        return FilePath(m_path.clone(), m_slashIndex);
     }
 
 private:
-    Utils::SmallString directory_;
-    Utils::SmallString name_;
+    Utils::PathString m_path = "/";
+    std::size_t m_slashIndex = 0;
 };
 
 CMBIPC_EXPORT QDebug operator<<(QDebug debug, const FilePath &filePath);

src/libs/clangbackendipc/sourcefilepathcontainerbase.h

@@ -42,13 +42,10 @@ public:
     {
     }
 
-    void insertFilePath(uint fileId, Utils::SmallString &&fileDirectory, Utils::SmallString &&fileName)
+    void insertFilePath(uint fileId, Utils::PathString &&filePath)
     {
-        if (m_filePathHash.find(fileId) == m_filePathHash.end()) {
-            m_filePathHash.emplace(std::piecewise_construct,
-                                 std::forward_as_tuple(fileId),
-                                 std::forward_as_tuple(std::move(fileDirectory), std::move(fileName)));
-        }
+        if (m_filePathHash.find(fileId) == m_filePathHash.end())
+            m_filePathHash.emplace(fileId, std::move(filePath));
     }
 
     void reserve(std::size_t size)

src/libs/clangbackendipc/sourcelocationcontainerv2.cpp

@@ -46,10 +46,10 @@ QDebug operator<<(QDebug debug, const SourceLocationContainer &container)
 std::ostream &operator<<(std::ostream &os, const SourceLocationContainer &container)
 {
     os << "("
+       << container.fileHash() << ", "
        << container.line() << ", "
        << container.column() << ", "
-       << container.offset() << ", "
-       << container.fileHash()
+       << container.offset()
        << ")";
 
     return os;

src/libs/clangbackendipc/sourcerangewithtextcontainer.h

@@ -110,6 +110,8 @@ private:
     Utils::SmallString m_text;
 };
 
+using SourceRangeWithTextContainers = std::vector<SourceRangeWithTextContainer>;
+
 CMBIPC_EXPORT QDebug operator<<(QDebug debug, const SourceRangeWithTextContainer &container);
 std::ostream &operator<<(std::ostream &os, const SourceRangeWithTextContainer &container);
 } // namespace ClangBackEnd

src/tools/clangpchmanagerbackend/source/stringcache.h → src/libs/clangbackendipc/stringcache.h

@@ -25,16 +25,26 @@
 
 #pragma once
 
-#include "clangpchmanagerbackend_global.h"
-
 #include <utils/smallstringview.h>
 
 #include <algorithm>
+#include <mutex>
 #include <vector>
 
 namespace ClangBackEnd {
 
-template <typename StringType>
+class NonLockingMutex
+{
+public:
+    constexpr NonLockingMutex() noexcept {}
+    NonLockingMutex(const NonLockingMutex&) = delete;
+    NonLockingMutex& operator=(const NonLockingMutex&) = delete;
+    void lock() {}
+    void unlock() {}
+};
+
+template <typename StringType,
+          typename Mutex = NonLockingMutex>
 class StringCache
 {
     class StringCacheEntry
@@ -79,6 +89,8 @@ public:
 
     uint stringId(Utils::SmallStringView stringView)
     {
+        std::lock_guard<Mutex> lock(m_mutex);
+
         Found found = find(stringView);
 
         if (!found.wasFound)
@@ -89,6 +101,8 @@ public:
 
     std::vector<uint> stringIds(const std::vector<StringType> &strings)
     {
+        std::lock_guard<Mutex> lock(m_mutex);
+
         std::vector<uint> ids;
         ids.reserve(strings.size());
 
@@ -102,11 +116,15 @@ public:
 
     const StringType &string(uint id) const
     {
+        std::lock_guard<Mutex> lock(m_mutex);
+
         return m_strings.at(m_indices.at(id)).string;
     }
 
     std::vector<StringType> strings(const std::vector<uint> &ids) const
     {
+        std::lock_guard<Mutex> lock(m_mutex);
+
         std::vector<StringType> strings;
         strings.reserve(ids.size());
 
@@ -155,6 +173,7 @@ private:
 private:
     StringCacheEntries m_strings;
     std::vector<uint> m_indices;
+    mutable Mutex m_mutex;
 };
 
 } // namespace ClangBackEnd

src/libs/utils/smallstringio.h

@@ -68,8 +68,8 @@ QDataStream &operator>>(QDataStream &in, BasicSmallString<Size> &string)
     return in;
 }
 
-inline
-QDebug &operator<<(QDebug &debug, const SmallString &string)
+template <typename String>
+QDebug &operator<<(QDebug &debug, const String &string)
 {
     using QT_PREPEND_NAMESPACE(operator<<);
 

src/libs/utils/smallstringview.h

@@ -104,6 +104,11 @@ public:
         return const_reverse_iterator(begin() - static_cast<std::size_t>(1));
     }
 
+    operator std::string() const
+    {
+        return std::string(data(), size());
+    }
+
 private:
     const char *m_pointer;
     size_type m_size;

src/plugins/clangrefactoring/refactoringclient.cpp

@@ -104,19 +104,6 @@ void RefactoringClient::setRefactoringConnectionClient(
     this->connectionClient = connectionClient;
 }
 
-namespace {
-
-Utils::SmallString concatenateFilePath(const ClangBackEnd::FilePath &filePath)
-{
-    Utils::SmallString concatenatedFilePath = filePath.directory().clone();
-    concatenatedFilePath.append("/");
-    concatenatedFilePath.append(filePath.name().clone());
-
-    return concatenatedFilePath;
-}
-
-}
-
 std::unordered_map<uint, QString> RefactoringClient::convertFilePaths(
         const ClangBackEnd::FilePathDict &filePaths)
 {
@@ -126,7 +113,7 @@ std::unordered_map<uint, QString> RefactoringClient::convertFilePaths(
 
     auto convertFilePath = [] (const ClangBackEnd::FilePathDict::value_type &dictonaryEntry) {
         return std::make_pair(dictonaryEntry.first,
-                              concatenateFilePath(dictonaryEntry.second).toQString());
+                              dictonaryEntry.second.path().toQString());
     };
 
     std::transform(filePaths.begin(),

src/tools/clangpchmanagerbackend/source/clangpchmanagerbackend-source.pri

@@ -19,7 +19,6 @@ HEADERS += \
     $$PWD/environment.h \
     $$PWD/clangpathwatcher.h \
     $$PWD/projectparts.h \
-    $$PWD/stringcache.h \
     $$PWD/idpaths.h \
     $$PWD/pchcreatorinterface.h \
     $$PWD/clangpathwatcherinterface.h \

src/tools/clangrefactoringbackend/source/clangquery.cpp

@@ -30,27 +30,15 @@
 
 #include <sourcerangescontainer.h>
 
-#include <QTime>
+#include <stringcache.h>
 
-#if defined(__GNUC__)
-#    pragma GCC diagnostic push
-#    pragma GCC diagnostic ignored "-Wunused-parameter"
-#elif defined(_MSC_VER)
-#    pragma warning(push)
-#    pragma warning( disable : 4100 )
-#endif
+#include <QTime>
 
 #include <clang/ASTMatchers/ASTMatchers.h>
 #include <clang/ASTMatchers/ASTMatchFinder.h>
 #include <clang/ASTMatchers/Dynamic/Diagnostics.h>
 #include <clang/ASTMatchers/Dynamic/Parser.h>
 
-#if defined(__GNUC__)
-#    pragma GCC diagnostic pop
-#elif defined(_MSC_VER)
-#    pragma warning(pop)
-#endif
-
 using clang::ast_matchers::dynamic::Diagnostics;
 using clang::ast_matchers::dynamic::Parser;
 using clang::ast_matchers::BoundNodes;
@@ -66,8 +54,10 @@ struct CollectBoundNodes : MatchFinder::MatchCallback {
   }
 };
 
-ClangQuery::ClangQuery(Utils::SmallString &&query)
-    : query(std::move(query))
+ClangQuery::ClangQuery(StringCache<Utils::PathString, std::mutex> &filePathCache,
+                       Utils::SmallString &&query)
+    : query(std::move(query)),
+      filePathCache(filePathCache)
 {
 }
 
@@ -226,6 +216,7 @@ void ClangQuery::matchLocation(
 
         SourceRangeExtractor extractor(ast->getSourceManager(),
                                        ast->getLangOpts(),
+                                       filePathCache,
                                        sourceRangesContainer);
         extractor.addSourceRanges(sourceRanges);
 

src/tools/clangrefactoringbackend/source/clangquery.h

@@ -30,6 +30,8 @@
 #include <sourcerangescontainer.h>
 #include <dynamicastmatcherdiagnosticcontainer.h>
 
+#include <stringcache.h>
+
 namespace clang {
 namespace ast_matchers {
 namespace dynamic {
@@ -49,7 +51,7 @@ namespace ClangBackEnd {
 class ClangQuery : public ClangTool
 {
 public:
-    ClangQuery(Utils::SmallString &&query={});
+    ClangQuery(StringCache<Utils::PathString, std::mutex> &filePathCache, Utils::SmallString &&query={});
 
     void setQuery(Utils::SmallString &&query);
 
@@ -67,6 +69,7 @@ private:
     SourceRangesContainer sourceRangesContainer;
     Utils::SmallString query;
     std::vector<DynamicASTMatcherDiagnosticContainer> diagnosticContainers_;
+    StringCache<Utils::PathString, std::mutex> &filePathCache;
 };
 
 } // namespace ClangBackEnd

src/tools/clangrefactoringbackend/source/clangquerygatherer.cpp

@@ -29,10 +29,12 @@
 
 namespace ClangBackEnd {
 
-ClangQueryGatherer::ClangQueryGatherer(std::vector<V2::FileContainer> &&sources,
+ClangQueryGatherer::ClangQueryGatherer(StringCache<Utils::PathString, std::mutex> *filePathCache,
+                                       std::vector<V2::FileContainer> &&sources,
                                        std::vector<V2::FileContainer> &&unsaved,
                                        Utils::SmallString &&query)
-    : m_sources(std::move(sources)),
+    : m_filePathCache(filePathCache),
+      m_sources(std::move(sources)),
       m_unsaved(std::move(unsaved)),
       m_query(std::move(query))
 {
@@ -40,11 +42,12 @@ ClangQueryGatherer::ClangQueryGatherer(std::vector<V2::FileContainer> &&sources,
 
 SourceRangesAndDiagnosticsForQueryMessage
 ClangQueryGatherer::createSourceRangesAndDiagnosticsForSource(
+        StringCache<Utils::PathString, std::mutex> *filePathCache,
         V2::FileContainer &&source,
         const std::vector<V2::FileContainer> &unsaved,
         Utils::SmallString &&query)
 {
-    ClangQuery clangQuery(std::move(query));
+    ClangQuery clangQuery(*filePathCache, std::move(query));
 
     clangQuery.addFile(source.filePath().directory(),
                        source.filePath().name(),
@@ -65,7 +68,8 @@ bool ClangQueryGatherer::canCreateSourceRangesAndDiagnostics() const
 
 SourceRangesAndDiagnosticsForQueryMessage ClangQueryGatherer::createNextSourceRangesAndDiagnostics()
 {
-    auto message = createSourceRangesAndDiagnosticsForSource(std::move(m_sources.back()),
+    auto message = createSourceRangesAndDiagnosticsForSource(m_filePathCache,
+                                                             std::move(m_sources.back()),
                                                              m_unsaved,
                                                              m_query.clone());
     m_sources.pop_back();
@@ -77,6 +81,7 @@ ClangQueryGatherer::Future ClangQueryGatherer::startCreateNextSourceRangesAndDia
 {
     Future future = std::async(std::launch::async,
                                createSourceRangesAndDiagnosticsForSource,
+                               m_filePathCache,
                                std::move(m_sources.back()),
                                m_unsaved,
                                m_query.clone());

src/tools/clangrefactoringbackend/source/clangquerygatherer.h

@@ -27,6 +27,7 @@
 
 #include <sourcerangesanddiagnosticsforquerymessage.h>
 #include <filecontainerv2.h>
+#include <stringcache.h>
 
 #include <future>
 
@@ -38,16 +39,16 @@ public:
     using Future = std::future<SourceRangesAndDiagnosticsForQueryMessage>;
 
     ClangQueryGatherer() = default;
-    ClangQueryGatherer(std::vector<V2::FileContainer> &&sources,
+    ClangQueryGatherer(StringCache<Utils::PathString, std::mutex> *filePathCache,
+                       std::vector<V2::FileContainer> &&sources,
                        std::vector<V2::FileContainer> &&unsaved,
                        Utils::SmallString &&query);
 
-    static
-    SourceRangesAndDiagnosticsForQueryMessage createSourceRangesAndDiagnosticsForSource(
+    static SourceRangesAndDiagnosticsForQueryMessage createSourceRangesAndDiagnosticsForSource(
+            StringCache<Utils::PathString, std::mutex> *filePathCache,
             V2::FileContainer &&source,
             const std::vector<V2::FileContainer> &unsaved,
             Utils::SmallString &&query);
-
     bool canCreateSourceRangesAndDiagnostics() const;
     SourceRangesAndDiagnosticsForQueryMessage createNextSourceRangesAndDiagnostics();
     Future startCreateNextSourceRangesAndDiagnosticsMessage();
@@ -66,6 +67,7 @@ protected:
     std::vector<Future> finishedFutures();
 
 private:
+    StringCache<Utils::PathString, std::mutex> *m_filePathCache = nullptr;
     std::vector<V2::FileContainer> m_sources;
     std::vector<V2::FileContainer> m_unsaved;
     Utils::SmallString m_query;

src/tools/clangrefactoringbackend/source/clangtool.cpp

@@ -84,23 +84,12 @@ template
 void ClangTool::addFiles<Utils::PathStringVector>(const Utils::PathStringVector &filePaths,
                                                   const Utils::SmallStringVector &arguments);
 
-namespace {
-Utils::SmallString toNativeFilePath(const FilePath &filePath)
-{
-    Utils::SmallString filePathString = filePath.directory().clone();
-    filePathString.append("/");
-    filePathString.append(filePath.name());
-
-    return toNativePath(std::move(filePathString));
-}
-}
-
 void ClangTool::addUnsavedFiles(const V2::FileContainers &unsavedFiles)
 {
     unsavedFileContents.reserve(unsavedFileContents.size() + unsavedFiles.size());
 
     auto convertToUnsavedFileContent = [] (const V2::FileContainer &unsavedFile) {
-        return UnsavedFileContent{toNativeFilePath(unsavedFile.filePath()),
+        return UnsavedFileContent{toNativePath(unsavedFile.filePath().path().clone()),
                                   unsavedFile.unsavedFileContent().clone()};
     };
 
@@ -111,7 +100,8 @@ void ClangTool::addUnsavedFiles(const V2::FileContainers &unsavedFiles)
 }
 
 namespace {
-llvm::StringRef toStringRef(const Utils::SmallString &string)
+template <typename String>
+llvm::StringRef toStringRef(const String &string)
 {
     return llvm::StringRef(string.data(), string.size());
 }

src/tools/clangrefactoringbackend/source/clangtool.h

@@ -77,13 +77,13 @@ struct FileContent
 
 struct UnsavedFileContent
 {
-    UnsavedFileContent(Utils::SmallString &&filePath,
+    UnsavedFileContent(Utils::PathString &&filePath,
                        Utils::SmallString &&content)
         : filePath(std::move(filePath)),
           content(std::move(content))
     {}
 
-    Utils::SmallString filePath;
+    Utils::PathString filePath;
     Utils::SmallString content;
 };
 

src/tools/clangrefactoringbackend/source/refactoringserver.cpp

@@ -37,6 +37,7 @@
 #include <QCoreApplication>
 
 #include <functional>
+#include <atomic>
 
 namespace ClangBackEnd {
 
@@ -125,7 +126,7 @@ void RefactoringServer::gatherSourceRangesAndDiagnosticsForQueryMessages(
     uint freeProcessors = std::thread::hardware_concurrency();
 #endif
 
-    m_gatherer = ClangQueryGatherer(std::move(sources), std::move(unsaved), std::move(query));
+    m_gatherer = ClangQueryGatherer(&m_filePathCache, std::move(sources), std::move(unsaved), std::move(query));
     m_gatherer.setProcessingSlotCount(freeProcessors);
 
     m_pollTimer.start();

src/tools/clangrefactoringbackend/source/refactoringserver.h

@@ -30,7 +30,12 @@
 #include <refactoringserverinterface.h>
 
 #include <QTimer>
+#include <stringcache.h>
 
+#include <utils/smallstring.h>
+
+#include <future>
+#include <mutex>
 #include <vector>
 
 namespace ClangBackEnd {
@@ -65,6 +70,7 @@ private:
                                                           Utils::SmallString &&query);
 
 private:
+    StringCache<Utils::PathString, std::mutex> m_filePathCache;
     ClangQueryGatherer m_gatherer;
     QTimer m_pollTimer;
 };

src/tools/clangrefactoringbackend/source/sourcelocationsutils.h

@@ -53,7 +53,7 @@
 namespace ClangBackEnd {
 
 inline
-llvm::SmallString<256> absolutePath(const llvm::StringRef &path)
+llvm::SmallString<256> absolutePath(clang::StringRef path)
 {
     llvm::SmallString<256> absolutePath(path);
 
@@ -64,9 +64,9 @@ llvm::SmallString<256> absolutePath(const llvm::StringRef &path)
 }
 
 template <typename Container>
-Utils::SmallString fromNativePath(Container container)
+Utils::PathString fromNativePath(Container container)
 {
-    Utils::SmallString path(container.data(), container.size());
+    Utils::PathString path(container.data(), container.size());
 
 #ifdef _WIN32
     std::replace(path.begin(), path.end(), '\\', '/');
@@ -89,13 +89,9 @@ void appendSourceLocationsToSourceLocationsContainer(
         const auto fileId = decomposedLoction.first;
         const auto offset = decomposedLoction.second;
         const auto fileEntry = sourceManager.getFileEntryForID(fileId);
-        auto filePath = absolutePath(fileEntry->getName());
-        const auto fileName = llvm::sys::path::filename(filePath);
-        llvm::sys::path::remove_filename(filePath);
 
         sourceLocationsContainer.insertFilePath(fileId.getHashValue(),
-                                                fromNativePath(filePath),
-                                                fromNativePath(fileName));
+                                                fromNativePath(fileEntry->tryGetRealPathName()));
         sourceLocationsContainer.insertSourceLocation(fileId.getHashValue(),
                                                       fullSourceLocation.getSpellingLineNumber(),