Utils: Cleanup StringCache

There is now a forward header and it is possible template the string type.

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

src/libs/clangbackendipc/clangbackendipc-lib.pri

@@ -171,6 +171,7 @@ HEADERS += \
     $$PWD/updatetranslationunitsforeditormessage.h \
     $$PWD/updatevisibletranslationunitsmessage.h \
     $$PWD/writemessageblock.h \
-    $$PWD/ipcclientprovider.h
+    $$PWD/ipcclientprovider.h \
+    $$PWD/stringcachefwd.h
 
 contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols

src/libs/clangbackendipc/stringcache.h

@@ -26,6 +26,7 @@
 #pragma once
 
 #include <utils/smallstringview.h>
+#include <utils/smallstringfwd.h>
 
 #include <algorithm>
 #include <mutex>
@@ -33,6 +34,14 @@
 
 namespace ClangBackEnd {
 
+class StringCacheException : public std::exception
+{
+    const char *what() const noexcept override
+    {
+        return "StringCache entries are in invalid state.";
+    }
+};
+
 class NonLockingMutex
 {
 public:
@@ -43,43 +52,50 @@ public:
     void unlock() {}
 };
 
-template <typename StringType,
-          typename Mutex = NonLockingMutex>
-class StringCache
+template <typename StringType>
+class StringCacheEntry
 {
-    class StringCacheEntry
+public:
+    StringCacheEntry(StringType &&string, uint id)
+        : string(std::move(string)),
+          id(id)
+    {}
+
+    friend bool operator<(const StringCacheEntry &entry, Utils::SmallStringView stringView)
     {
-    public:
-        StringCacheEntry(StringType &&string, uint id)
-            : string(std::move(string)),
-              id(id)
-        {}
-
-        friend bool operator<(const StringCacheEntry &entry, Utils::SmallStringView stringView)
-        {
-            return entry.string < stringView;
-        }
+        return entry.string < stringView;
+    }
 
-        friend bool operator<(Utils::SmallStringView stringView, const StringCacheEntry &entry)
-        {
-            return stringView < entry.string;
-        }
+    friend bool operator<(Utils::SmallStringView stringView, const StringCacheEntry &entry)
+    {
+        return stringView < entry.string;
+    }
 
-        StringType string;
-        uint id;
-    };
+    friend bool operator<(const StringCacheEntry &first, const StringCacheEntry &second)
+    {
+        return first.string < second.string;
+    }
+
+    StringType string;
+    uint id;
+};
 
-    using StringCacheEntries = std::vector<StringCacheEntry>;
-    using const_iterator = typename StringCacheEntries::const_iterator;
+template <typename StringType>
+using StringCacheEntries = std::vector<StringCacheEntry<StringType>>;
+
+template <typename StringType,
+          typename Mutex = NonLockingMutex>
+class StringCache
+{
+    using const_iterator = typename StringCacheEntries<StringType>::const_iterator;
 
     class Found
     {
     public:
-        typename StringCacheEntries::const_iterator iterator;
+        typename StringCacheEntries<StringType>::const_iterator iterator;
         bool wasFound;
     };
 
-
 public:
     StringCache()
     {
@@ -87,6 +103,26 @@ public:
         m_indices.reserve(1024);
     }
 
+    void populate(StringCacheEntries<StringType> &&entries)
+    {
+        uncheckedPopulate(std::move(entries));
+
+        checkEntries();
+    }
+
+    void uncheckedPopulate(StringCacheEntries<StringType> &&entries)
+    {
+        std::sort(entries.begin(), entries.end());
+
+        m_strings = std::move(entries);
+        m_indices.resize(m_strings.size());
+
+        auto begin = m_strings.cbegin();
+        for (auto current = begin; current != m_strings.end(); ++current)
+            m_indices.at(current->id) = std::distance(begin, current);
+    }
+
+
     uint stringId(Utils::SmallStringView stringView)
     {
         std::lock_guard<Mutex> lock(m_mutex);
@@ -99,7 +135,8 @@ public:
         return found.iterator->id;
     }
 
-    std::vector<uint> stringIds(const std::vector<StringType> &strings)
+    template <typename Container>
+    std::vector<uint> stringIds(const Container &strings)
     {
         std::lock_guard<Mutex> lock(m_mutex);
 
@@ -109,12 +146,17 @@ public:
         std::transform(strings.begin(),
                        strings.end(),
                        std::back_inserter(ids),
-                       [&] (const StringType &string) { return stringId(string); });
+                       [&] (const auto &string) { return this->stringId(string); });
 
         return ids;
     }
 
-    const StringType &string(uint id) const
+    std::vector<uint> stringIds(std::initializer_list<StringType> strings)
+    {
+        return stringIds<std::initializer_list<StringType>>(strings);
+    }
+
+    Utils::SmallStringView string(uint id) const
     {
         std::lock_guard<Mutex> lock(m_mutex);
 
@@ -136,6 +178,11 @@ public:
         return strings;
     }
 
+    bool isEmpty() const
+    {
+        return m_strings.empty() && m_indices.empty();
+    }
+
 private:
     Found find(Utils::SmallStringView stringView)
     {
@@ -157,7 +204,7 @@ private:
     uint insertString(const_iterator beforeIterator,
                       Utils::SmallStringView stringView)
     {
-        auto id = uint(m_strings.size());
+        auto id = uint(m_indices.size());
 
         auto inserted = m_strings.emplace(beforeIterator, StringType(stringView), id);
 
@@ -170,10 +217,21 @@ private:
         return id;
     }
 
+    void checkEntries()
+    {
+        for (const auto &entry : m_strings) {
+            if (entry.string != string(entry.id) || entry.id != stringId(entry.string))
+                throw StringCacheException();
+        }
+    }
+
 private:
-    StringCacheEntries m_strings;
+    StringCacheEntries<StringType> m_strings;
     std::vector<uint> m_indices;
     mutable Mutex m_mutex;
 };
 
+template <typename Mutex = NonLockingMutex>
+using FilePathCache = StringCache<Utils::PathString, Mutex>;
+
 } // namespace ClangBackEnd

src/libs/clangbackendipc/stringcachefwd.h

+/****************************************************************************
+**
+** Copyright (C) 2017 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+#pragma once
+
+#include <utils/smallstringfwd.h>
+
+namespace ClangBackEnd {
+
+class NonLockingMutex;
+
+template <typename StringType,
+          typename Mutex>
+class StringCache;
+
+template <typename Mutex = NonLockingMutex>
+using FilePathCache = StringCache<Utils::PathString, Mutex>;
+
+} // namespace ClangBackEnd
+

src/tools/clangpchmanagerbackend/source/clangpathwatcher.h

@@ -216,7 +216,7 @@ unittest_public:
         std::transform(watcherEntries.begin(),
                        watcherEntries.end(),
                        std::back_inserter(paths),
-                       [&] (const WatcherEntry &entry) { return m_pathCache.string(entry.path); });
+                       [&] (const WatcherEntry &entry) { return QString(m_pathCache.string(entry.path)); });
 
         return paths;
     }
@@ -388,7 +388,7 @@ unittest_public:
                        foundEntries.end(),
                        std::back_inserter(ids),
                        [&] (const WatcherEntry &entry) {
-            return m_idCache.string(entry.id);
+            return Utils::SmallString(m_idCache.string(entry.id));
         });
 
         return ids;

tests/unit/unittest/stringcache-test.cpp

@@ -31,7 +31,8 @@
 
 namespace {
 
-using testing::ElementsAre;
+using ClangBackEnd::StringCacheEntries;
+using ClangBackEnd::StringCacheException;
 
 class StringCache : public testing::Test
 {
@@ -125,4 +126,84 @@ TEST_F(StringCache, AddFilePaths)
     ASSERT_THAT(ids, ElementsAre(0, 1, 2, 3));
 }
 
+TEST_F(StringCache, IsEmpty)
+{
+    auto isEmpty = cache.isEmpty();
+
+    ASSERT_TRUE(isEmpty);
+}
+
+TEST_F(StringCache, IsNotEmpty)
+{
+    cache.stringId(filePath1);
+
+    auto isEmpty = cache.isEmpty();
+
+    ASSERT_FALSE(isEmpty);
+}
+
+TEST_F(StringCache, PopulateWithEmptyVector)
+{
+    StringCacheEntries<Utils::PathString> entries;
+
+    cache.uncheckedPopulate(std::move(entries));
+
+    ASSERT_TRUE(cache.isEmpty());
+}
+
+TEST_F(StringCache, IsNotEmptyAfterPopulateWithSomeEntries)
+{
+    StringCacheEntries<Utils::PathString> entries{{filePath1.clone(), 0},
+                                                  {filePath2.clone(), 1},
+                                                  {filePath3.clone(), 2},
+                                                  {filePath4.clone(), 3}};
+
+    cache.uncheckedPopulate(std::move(entries));
+
+    ASSERT_TRUE(!cache.isEmpty());
+}
+
+TEST_F(StringCache, GetEntryAfterPopulateWithSomeEntries)
+{
+    StringCacheEntries<Utils::PathString> entries{{filePath1.clone(), 0},
+                                                  {filePath2.clone(), 1},
+                                                  {filePath3.clone(), 2},
+                                                  {filePath4.clone(), 3}};
+    cache.uncheckedPopulate(std::move(entries));
+
+    auto string = cache.string(2);
+
+    ASSERT_THAT(string, filePath3);
+}
+
+TEST_F(StringCache, EntriesHaveUniqueIds)
+{
+    StringCacheEntries<Utils::PathString> entries{{filePath1.clone(), 0},
+                                                  {filePath2.clone(), 1},
+                                                  {filePath3.clone(), 2},
+                                                  {filePath4.clone(), 2}};
+
+    ASSERT_THROW(cache.populate(std::move(entries)), StringCacheException);
+}
+
+TEST_F(StringCache, IdsAreHigherLowerEntriesSize)
+{
+    StringCacheEntries<Utils::PathString> entries{{filePath1.clone(), 0},
+                                                  {filePath2.clone(), 1},
+                                                  {filePath3.clone(), 4},
+                                                  {filePath4.clone(), 3}};
+
+    ASSERT_THROW(cache.populate(std::move(entries)), std::out_of_range);
+}
+
+TEST_F(StringCache, MultipleEntries)
+{
+    StringCacheEntries<Utils::PathString> entries{{filePath1.clone(), 0},
+                                                  {filePath1.clone(), 1},
+                                                  {filePath3.clone(), 2},
+                                                  {filePath4.clone(), 3}};
+
+    ASSERT_THROW(cache.populate(std::move(entries)), StringCacheException);
+}
+
 }