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

@@ -70,6 +70,7 @@ public:
 using WatcherEntries = std::vector<WatcherEntry>;
 
 using IdCache = StringCache<Utils::SmallString,
+                            Utils::SmallStringView,
                             int,
                             NonLockingMutex,
                             decltype(&Utils::compare),

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),
+                                      decltype(&FileNameView::compare),
-                                      Utils::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,
+        int fileNameId = m_fileNameCache.stringId({fileName, directoryId},
-                                                  [&] (const Utils::SmallStringView) {
+                                                  [&] (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);

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>
+template <typename StringType,
-using StringCacheEntries = std::vector<StringCacheEntry<StringType, IndexType>>;
+          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 CacheEntry = StringCacheEntry<StringType, StringViewType, IndexType>;
-    using CacheEntries = StringCacheEntries<StringType, 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);

src/libs/clangsupport/stringcachefwd.h

@@ -32,6 +32,7 @@ namespace ClangBackEnd {
 class NonLockingMutex;
 
 template <typename StringType,
+          typename StringViewType,
           typename IndexType,
           typename Mutex,
           typename Compare,

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))

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),