From ff1ce3a475a22e3a9787eda5f893591906ed1af0 Mon Sep 17 00:00:00 2001
From: Marco Bubke <marco.bubke@qt.io>
Date: Tue, 27 Mar 2018 14:23:42 +0200
Subject: [PATCH] Clang: Honor directories for the file name

The file name id must be unique for very entry, so the directory id must be
incorporated too. Now there is always one unique integer id for every
file path. The directory id is there to access and compare the directory
much faster but not provide any data to the uniqueness of the id.

Change-Id: I0f9a2ca70bc9dda0ce32ebc45eb7b082821eb909
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>
---
 src/libs/clangsupport/clangpathwatcher.h   |  1 +
 src/libs/clangsupport/filepathcache.h      | 75 ++++++++++++++++++++--
 src/libs/clangsupport/stringcache.h        | 30 +++++----
 src/libs/clangsupport/stringcachefwd.h     |  1 +
 tests/unit/unittest/filepathcache-test.cpp | 22 +++++++
 tests/unit/unittest/stringcache-test.cpp   |  1 +
 6 files changed, 111 insertions(+), 19 deletions(-)

diff --git a/src/libs/clangsupport/clangpathwatcher.h b/src/libs/clangsupport/clangpathwatcher.h
index 9339c56d51d..258c63e9383 100644
--- a/src/libs/clangsupport/clangpathwatcher.h
+++ b/src/libs/clangsupport/clangpathwatcher.h
@@ -70,6 +70,7 @@ public:
 using WatcherEntries = std::vector<WatcherEntry>;
 
 using IdCache = StringCache<Utils::SmallString,
+                            Utils::SmallStringView,
                             int,
                             NonLockingMutex,
                             decltype(&Utils::compare),
diff --git a/src/libs/clangsupport/filepathcache.h b/src/libs/clangsupport/filepathcache.h
index 0e293156012..92e7892012b 100644
--- a/src/libs/clangsupport/filepathcache.h
+++ b/src/libs/clangsupport/filepathcache.h
@@ -38,16 +38,77 @@ namespace ClangBackEnd {
 template <typename FilePathStorage>
 class FilePathCache
 {
+    class FileNameView
+    {
+    public:
+        friend bool operator==(const FileNameView &first, const FileNameView &second)
+        {
+            return first.directoryId == second.directoryId
+                && first.fileName == second.fileName;
+        }
+
+        static
+        int compare(FileNameView first, FileNameView second) noexcept
+        {
+            int directoryDifference = first.directoryId - second.directoryId;
+
+            if (directoryDifference)
+                return directoryDifference;
+
+            return Utils::compare(first.fileName, second.fileName);
+        }
+
+    public:
+        Utils::SmallStringView fileName;
+        int directoryId;
+    };
+
+    class FileNameEntry
+    {
+    public:
+        FileNameEntry(Utils::SmallStringView fileName, int directoryId)
+            : fileName(fileName),
+              directoryId(directoryId)
+        {}
+
+        FileNameEntry(FileNameView view)
+            : fileName(view.fileName),
+              directoryId(view.directoryId)
+        {}
+
+        friend bool operator==(const FileNameEntry &first, const FileNameEntry &second)
+        {
+            return first.directoryId == second.directoryId
+                && first.fileName == second.fileName;
+        }
+
+        operator FileNameView() const
+        {
+            return {fileName, directoryId};
+        }
+
+        operator Utils::SmallString() &&
+        {
+            return std::move(fileName);
+        }
+
+    public:
+        Utils::SmallString fileName;
+        int directoryId;
+    };
+
     using DirectoryPathCache = StringCache<Utils::PathString,
+                                           Utils::SmallStringView,
                                            int,
                                            SharedMutex,
                                            decltype(&Utils::reverseCompare),
                                            Utils::reverseCompare>;
-    using FileNameCache = StringCache<Utils::SmallString,
+    using FileNameCache = StringCache<FileNameEntry,
+                                      FileNameView,
                                       int,
                                       SharedMutex,
-                                      decltype(&Utils::compare),
-                                      Utils::compare>;
+                                      decltype(&FileNameView::compare),
+                                      FileNameView::compare>;
 public:
     FilePathCache(FilePathStorage &filePathStorage)
         : m_filePathStorage(filePathStorage)
@@ -67,8 +128,8 @@ public:
 
         Utils::SmallStringView fileName = filePath.name();
 
-        int fileNameId = m_fileNameCache.stringId(fileName,
-                                                  [&] (const Utils::SmallStringView) {
+        int fileNameId = m_fileNameCache.stringId({fileName, directoryId},
+                                                  [&] (const FileNameView) {
             return m_filePathStorage.fetchSourceId(directoryId, fileName);
         });
 
@@ -86,7 +147,9 @@ public:
                                                                     fetchFilePath);
 
 
-        auto fetchSoureName = [&] (int id) { return m_filePathStorage.fetchSourceName(id); };
+        auto fetchSoureName = [&] (int id) {
+            return FileNameEntry{m_filePathStorage.fetchSourceName(id), filePathId.directoryId};
+        };
 
         Utils::SmallString fileName = m_fileNameCache.string(filePathId.filePathId,
                                                              fetchSoureName);
diff --git a/src/libs/clangsupport/stringcache.h b/src/libs/clangsupport/stringcache.h
index 2c9f0159330..9192b46aa52 100644
--- a/src/libs/clangsupport/stringcache.h
+++ b/src/libs/clangsupport/stringcache.h
@@ -29,7 +29,6 @@
 #include "stringcachefwd.h"
 
 #include <utils/optional.h>
-#include <utils/smallstringview.h>
 #include <utils/smallstringfwd.h>
 
 #include <QReadWriteLock>
@@ -91,7 +90,9 @@ private:
     QReadWriteLock m_mutex;
 };
 
-template <typename StringType, typename IndexType>
+template <typename StringType,
+          typename StringViewType,
+          typename IndexType>
 class StringCacheEntry
 {
 public:
@@ -100,19 +101,22 @@ public:
           id(id)
     {}
 
-    operator Utils::SmallStringView() const
+    operator StringViewType() const
     {
-        return {string.data(), string.size()};
+        return string;
     }
 
     StringType string;
     IndexType id;
 };
 
-template <typename StringType, typename IndexType>
-using StringCacheEntries = std::vector<StringCacheEntry<StringType, IndexType>>;
+template <typename StringType,
+          typename StringViewType,
+          typename IndexType>
+using StringCacheEntries = std::vector<StringCacheEntry<StringType, StringViewType, IndexType>>;
 
 template <typename StringType,
+          typename StringViewType,
           typename IndexType,
           typename Mutex,
           typename Compare,
@@ -120,8 +124,8 @@ template <typename StringType,
 class StringCache
 {
 public:
-    using CacheEntry = StringCacheEntry<StringType, IndexType>;
-    using CacheEntries = StringCacheEntries<StringType, IndexType>;
+    using CacheEntry = StringCacheEntry<StringType, StringViewType, IndexType>;
+    using CacheEntries = StringCacheEntries<StringType, StringViewType, IndexType>;
     using const_iterator = typename CacheEntries::const_iterator;
     using Found = ClangBackEnd::Found<const_iterator>;
 
@@ -145,7 +149,7 @@ public:
     {
         std::sort(entries.begin(),
                   entries.end(),
-                  [] (Utils::SmallStringView first, Utils::SmallStringView second) {
+                  [] (StringViewType first, StringViewType second) {
             return compare(first, second) < 0;
         });
 
@@ -158,7 +162,7 @@ public:
     }
 
 
-    IndexType stringId(Utils::SmallStringView stringView)
+    IndexType stringId(StringViewType stringView)
     {
         std::shared_lock<Mutex> sharedLock(m_mutex);
         Found found = find(stringView);
@@ -244,7 +248,7 @@ public:
     }
 
     template<typename Function>
-    IndexType stringId(Utils::SmallStringView stringView, Function storageFunction)
+    IndexType stringId(StringViewType stringView, Function storageFunction)
     {
         std::shared_lock<Mutex> sharedLock(m_mutex);
 
@@ -271,7 +275,7 @@ public:
     }
 
 private:
-    Found find(Utils::SmallStringView stringView)
+    Found find(StringViewType stringView)
     {
         return findInSorted(m_strings.cbegin(), m_strings.cend(), stringView, compare);
     }
@@ -293,7 +297,7 @@ private:
     }
 
     IndexType insertString(const_iterator beforeIterator,
-                           Utils::SmallStringView stringView,
+                           StringViewType stringView,
                            IndexType id)
     {
         auto inserted = m_strings.emplace(beforeIterator, StringType(stringView), id);
diff --git a/src/libs/clangsupport/stringcachefwd.h b/src/libs/clangsupport/stringcachefwd.h
index 75f03056b20..adc00ff1f5f 100644
--- a/src/libs/clangsupport/stringcachefwd.h
+++ b/src/libs/clangsupport/stringcachefwd.h
@@ -32,6 +32,7 @@ namespace ClangBackEnd {
 class NonLockingMutex;
 
 template <typename StringType,
+          typename StringViewType,
           typename IndexType,
           typename Mutex,
           typename Compare,
diff --git a/tests/unit/unittest/filepathcache-test.cpp b/tests/unit/unittest/filepathcache-test.cpp
index 8977ef645c1..282654ad7b1 100644
--- a/tests/unit/unittest/filepathcache-test.cpp
+++ b/tests/unit/unittest/filepathcache-test.cpp
@@ -156,14 +156,36 @@ TEST_F(FilePathCache, GetAFilePathWithCachedFilePathId)
     ASSERT_THAT(filePath, Eq(FilePathView{"/path/to/file.cpp"}));
 }
 
+TEST_F(FilePathCache, FileNamesAreUniqueForEveryDirectory)
+{
+    FilePathId filePathId = cache.filePathId(FilePathView("/path/to/file.cpp"));
+
+    FilePathId filePath2Id = cache.filePathId(FilePathView("/path2/to/file.cpp"));
+
+    ASSERT_THAT(filePath2Id.filePathId, Ne(filePathId.filePathId));
+}
+
+TEST_F(FilePathCache, DuplicateFilePathsAreEqual)
+{
+    FilePathId filePath1Id = cache.filePathId(FilePathView("/path/to/file.cpp"));
+
+    FilePathId filePath2Id = cache.filePathId(FilePathView("/path/to/file.cpp"));
+
+    ASSERT_THAT(filePath2Id, Eq(filePath1Id));
+}
+
 void FilePathCache::SetUp()
 {
     ON_CALL(mockStorage, fetchDirectoryId(Eq("/path/to")))
             .WillByDefault(Return(5));
+    ON_CALL(mockStorage, fetchDirectoryId(Eq("/path2/to")))
+            .WillByDefault(Return(6));
     ON_CALL(mockStorage, fetchSourceId(5, Eq("file.cpp")))
             .WillByDefault(Return(42));
     ON_CALL(mockStorage, fetchSourceId(5, Eq("file2.cpp")))
             .WillByDefault(Return(63));
+    ON_CALL(mockStorage, fetchSourceId(6, Eq("file.cpp")))
+            .WillByDefault(Return(72));
     ON_CALL(mockStorage, fetchDirectoryPath(5))
             .WillByDefault(Return(Utils::PathString("/path/to")));
     ON_CALL(mockStorage, fetchSourceName(42))
diff --git a/tests/unit/unittest/stringcache-test.cpp b/tests/unit/unittest/stringcache-test.cpp
index 8465321c34d..102d8510427 100644
--- a/tests/unit/unittest/stringcache-test.cpp
+++ b/tests/unit/unittest/stringcache-test.cpp
@@ -45,6 +45,7 @@ using StorageIdFunction = std::function<int(Utils::SmallStringView)>;
 using StorageStringFunction = std::function<Utils::PathString(int)>;
 
 using Cache = ClangBackEnd::StringCache<Utils::PathString,
+                                        Utils::SmallStringView,
                                         int,
                                         NiceMock<MockMutex>,
                                         decltype(&Utils::reverseCompare),