Feature/string hash review (#1142)

* * Added ConstArrayView
* Made StringSlicePool have styles
* Remove point about strings not having terminating 0 (they do), and restriction around ""

* spCalcStringHash -> spComputeStringHash

* Small code improvements.
Closer to coding conventions.

* Fix small bug with Empty adding c string.

* Fix typo in assert.

* Fix ArrayView compiling issue on gcc/clang.

* Remove tabs.

* Improve comments around StringSlicePool.
Simplify getting the added slices.

12 files changed, 44 insertions, 33 deletions

diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index 15121babf..67b4be449 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -250,7 +250,7 @@ DIAGNOSTIC(30049, Note,  thisIsImmutableByDefault, "a 'this' parameter is an imm
 
 DIAGNOSTIC(30051, Error, invalidValueForArgument, "invalid value for argument '$0'")
 DIAGNOSTIC(30052, Error, invalidSwizzleExpr, "invalid swizzle pattern '$0' on type '$1'")
-DIAGNOSTIC(30043, Error, getStringHashRequiresStringLiteral, "getStringHash parameter can only accept a non-zero length string literal")
+DIAGNOSTIC(30043, Error, getStringHashRequiresStringLiteral, "getStringHash parameter can only accept a string literal")
 
 DIAGNOSTIC(30060, Error, expectedAType, "expected a type got a '$0'")
 
diff --git a/source/slang/slang-emit-cpp.cpp b/source/slang/slang-emit-cpp.cpp
index 5592ba26a..e7f11fc92 100644
--- a/source/slang/slang-emit-cpp.cpp
+++ b/source/slang/slang-emit-cpp.cpp
@@ -1785,7 +1785,8 @@ void CPPSourceEmitter::emitOperationCall(IntrinsicOp op, IRInst* inst, IRUse* op
 /* !!!!!!!!!!!!!!!!!!!!!! CPPSourceEmitter !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
 
 CPPSourceEmitter::CPPSourceEmitter(const Desc& desc):
-    Super(desc)
+    Super(desc),
+    m_slicePool(StringSlicePool::Style::Default)
 {
     m_semanticUsedFlags = 0;
     //m_semanticUsedFlags = SemanticUsedFlag::GroupID | SemanticUsedFlag::GroupThreadID | SemanticUsedFlag::DispatchThreadID;
diff --git a/source/slang/slang-ir-link.cpp b/source/slang/slang-ir-link.cpp
index 0678b5962..6399e3f6b 100644
--- a/source/slang/slang-ir-link.cpp
+++ b/source/slang/slang-ir-link.cpp
@@ -1393,7 +1393,7 @@ LinkedIR linkIR(
 
     // Combine all of the contents of IRGlobalHashedStringLiterals
     {
-        StringSlicePool pool;
+        StringSlicePool pool(StringSlicePool::Style::Empty);
         IRBuilder& builder = sharedContext->builderStorage;
         for (IRModule* irModule : irModules)
         {
diff --git a/source/slang/slang-ir-serialize-types.cpp b/source/slang/slang-ir-serialize-types.cpp
index 998fc9d7f..10e2776b2 100644
--- a/source/slang/slang-ir-serialize-types.cpp
+++ b/source/slang/slang-ir-serialize-types.cpp
@@ -77,8 +77,8 @@ void StringRepresentationCache::init(const List<char>* stringTable, NamePool* na
     m_scopeManager = scopeManager;
 
     // Decode the table
-    m_entries.setCount(StringSlicePool::kNumDefaultHandles);
-    SLANG_COMPILE_TIME_ASSERT(StringSlicePool::kNumDefaultHandles == 2);
+    m_entries.setCount(StringSlicePool::kDefaultHandlesCount);
+    SLANG_COMPILE_TIME_ASSERT(StringSlicePool::kDefaultHandlesCount == 2);
 
     {
         Entry& entry = m_entries[0];
@@ -200,15 +200,14 @@ char* StringRepresentationCache::getCStr(Handle handle)
 /* static */void SerialStringTableUtil::encodeStringTable(const StringSlicePool& pool, List<char>& stringTable)
 {
     // Skip the default handles -> nothing is encoded via them
-    return encodeStringTable(pool.getSlices().begin() + StringSlicePool::kNumDefaultHandles, pool.getNumSlices() - StringSlicePool::kNumDefaultHandles, stringTable);
+    return encodeStringTable(pool.getAdded(), stringTable);
 }
     
-/* static */void SerialStringTableUtil::encodeStringTable(const UnownedStringSlice* slices, size_t numSlices, List<char>& stringTable)
+/* static */void SerialStringTableUtil::encodeStringTable(const ConstArrayView<UnownedStringSlice>& slices, List<char>& stringTable)
 {
     stringTable.clear();
-    for (size_t i = 0; i < numSlices; ++i)
+    for (const auto& slice : slices)
     {
-        const UnownedStringSlice slice = slices[i];
         const int len = int(slice.size());
         
         // We need to write into the the string array
@@ -251,19 +250,19 @@ char* StringRepresentationCache::getCStr(Handle handle)
 
 /* static */void SerialStringTableUtil::calcStringSlicePoolMap(const List<UnownedStringSlice>& slices, StringSlicePool& pool, List<StringSlicePool::Handle>& indexMapOut)
 {
-    SLANG_ASSERT(slices.getCount() >= StringSlicePool::kNumDefaultHandles);
+    SLANG_ASSERT(slices.getCount() >= StringSlicePool::kDefaultHandlesCount);
     SLANG_ASSERT(slices[int(StringSlicePool::kNullHandle)] == "" && slices[int(StringSlicePool::kNullHandle)].begin() == nullptr);
     SLANG_ASSERT(slices[int(StringSlicePool::kEmptyHandle)] == "");
 
     indexMapOut.setCount(slices.getCount());
     // Set up all of the defaults
-    for (int i = 0; i < StringSlicePool::kNumDefaultHandles; ++i)
+    for (int i = 0; i < StringSlicePool::kDefaultHandlesCount; ++i)
     {
         indexMapOut[i] = StringSlicePool::Handle(i);
     }
 
     const Index numSlices = slices.getCount();
-    for (Index i = StringSlicePool::kNumDefaultHandles; i < numSlices ; ++i)
+    for (Index i = StringSlicePool::kDefaultHandlesCount; i < numSlices ; ++i)
     {
         indexMapOut[i] = pool.add(slices[i]);
     }
diff --git a/source/slang/slang-ir-serialize-types.h b/source/slang/slang-ir-serialize-types.h
index 0de9769c5..f4340e37e 100644
--- a/source/slang/slang-ir-serialize-types.h
+++ b/source/slang/slang-ir-serialize-types.h
@@ -5,6 +5,7 @@
 #include "../core/slang-object-scope-manager.h"
 #include "../core/slang-riff.h"
 #include "../core/slang-string-slice-pool.h"
+#include "../core/slang-array-view.h"
 
 #include "slang-name.h"
 #include "slang-source-loc.h"
@@ -59,7 +60,7 @@ struct SerialStringTableUtil
 {
         /// Convert a pool into a string table
     static void encodeStringTable(const StringSlicePool& pool, List<char>& stringTable);
-    static void encodeStringTable(const UnownedStringSlice* slices, size_t numSlices, List<char>& stringTable);
+    static void encodeStringTable(const ConstArrayView<UnownedStringSlice>& slices, List<char>& stringTable);
         /// Appends the decoded strings into slicesOut
     static void appendDecodedStringTable(const List<char>& stringTable, List<UnownedStringSlice>& slicesOut);
         /// Decodes a string table (and does so such that the indices are compatible with StringSlicePool)
diff --git a/source/slang/slang-ir-serialize.cpp b/source/slang/slang-ir-serialize.cpp
index a6ed35cc6..5d78f76f8 100644
--- a/source/slang/slang-ir-serialize.cpp
+++ b/source/slang/slang-ir-serialize.cpp
@@ -85,9 +85,12 @@ void IRSerialWriter::_addDebugSourceLocRun(SourceLoc sourceLoc, uint32_t startIn
             adjustedLineInfo.m_lineInfo = lineInfo;
             adjustedLineInfo.m_pathStringIndex = Ser::kNullStringIndex;
 
-            if (StringSlicePool::hasContents(entry.m_pathHandle))
+            const auto& pool = sourceView->getSourceManager()->getStringSlicePool();
+            SLANG_ASSERT(pool.getStyle() == StringSlicePool::Style::Default);
+
+            if (!pool.isDefaultHandle(entry.m_pathHandle))
             {
-                UnownedStringSlice slice = sourceView->getSourceManager()->getStringSlicePool().getSlice(entry.m_pathHandle);
+                UnownedStringSlice slice = pool.getSlice(entry.m_pathHandle);
                 SLANG_ASSERT(slice.size() > 0);
                 adjustedLineInfo.m_pathStringIndex = Ser::StringIndex(m_debugStringSlicePool.add(slice));
             }
diff --git a/source/slang/slang-ir-serialize.h b/source/slang/slang-ir-serialize.h
index 7e80aeaad..94c0666bb 100644
--- a/source/slang/slang-ir-serialize.h
+++ b/source/slang/slang-ir-serialize.h
@@ -56,7 +56,9 @@ struct IRSerialWriter
     StringSlicePool& getDebugStringPool() { return m_debugStringSlicePool; }
 
     IRSerialWriter() :
-        m_serialData(nullptr)
+        m_serialData(nullptr),
+        m_stringSlicePool(StringSlicePool::Style::Default),
+        m_debugStringSlicePool(StringSlicePool::Style::Default)
     {}
 
 protected:
diff --git a/source/slang/slang-ir-string-hash.cpp b/source/slang/slang-ir-string-hash.cpp
index b9e2d4045..0a15ba52d 100644
--- a/source/slang/slang-ir-string-hash.cpp
+++ b/source/slang/slang-ir-string-hash.cpp
@@ -47,7 +47,7 @@ void replaceGetStringHash(IRModule* module, SharedIRBuilder& sharedBuilder, Stri
 
 void replaceGetStringHashWithGlobal(IRModule* module, SharedIRBuilder& sharedBuilder)
 {
-    StringSlicePool pool;
+    StringSlicePool pool(StringSlicePool::Style::Empty);
     replaceGetStringHash(module, sharedBuilder, pool);
     addGlobalHashedStringLiterals(pool, sharedBuilder);
 }
@@ -56,7 +56,7 @@ void findGlobalHashedStringLiterals(IRModule* module, StringSlicePool& pool)
 {
     IRModuleInst* moduleInst = module->getModuleInst();
 
-    for (IRInst* child = moduleInst->getFirstDecorationOrChild(); child; child = child->getNextInst())
+    for(IRInst* child : moduleInst->getChildren())
     {
         if (IRGlobalHashedStringLiterals* hashedStringLits = as<IRGlobalHashedStringLiterals>(child))
         {
@@ -72,7 +72,8 @@ void findGlobalHashedStringLiterals(IRModule* module, StringSlicePool& pool)
 
 void addGlobalHashedStringLiterals(const StringSlicePool& pool, SharedIRBuilder& sharedBuilder)
 {
-    if (pool.getNumSlices() <= StringSlicePool::kNumDefaultHandles)
+    auto slices = pool.getAdded();
+    if (slices.getCount() == 0)
     {
         return;
     }
@@ -86,18 +87,16 @@ void addGlobalHashedStringLiterals(const StringSlicePool& pool, SharedIRBuilder&
     // We need to add a global instruction that references all of these string literals
     builder.setInsertInto(module->getModuleInst());
 
-    Index numSlices = Index(pool.getNumSlices() - StringSlicePool::kNumDefaultHandles);
+    const Index slicesCount = slices.getCount();
 
-    IRInst* globalHashedInst = createEmptyInst(module, kIROp_GlobalHashedStringLiterals, int(numSlices));
+    IRInst* globalHashedInst = createEmptyInst(module, kIROp_GlobalHashedStringLiterals, int(slicesCount));
     builder.addInst(globalHashedInst);
 
     auto operands = globalHashedInst->getOperands();
 
-    for (Index i = 0; i < numSlices; ++i)
+    for (Index i = 0; i < slicesCount; ++i)
     {
-        UnownedStringSlice slice = pool.getSlice(StringSlicePool::Handle(i + StringSlicePool::kNumDefaultHandles));
-        IRStringLit* stringLit = builder.getStringValue(slice);
-
+        IRStringLit* stringLit = builder.getStringValue(slices[i]);
         operands[i].init(globalHashedInst, stringLit);
     }
 
diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp
index 8b8a67f1e..2ea4f3d32 100644
--- a/source/slang/slang-lower-to-ir.cpp
+++ b/source/slang/slang-lower-to-ir.cpp
@@ -783,13 +783,10 @@ LoweredValInfo emitCallToDeclRef(
                 case kIROp_GetStringHash:
                 {
                     IRStringLit* stringLit = as<IRStringLit>(args[0]);
-
-                    if (stringLit == nullptr || stringLit->getStringSlice() == UnownedStringSlice())
+                    if (stringLit == nullptr)
                     {
                         auto sink = context->getSink();
-
                         sink->diagnose(funcDecl, Diagnostics::getStringHashRequiresStringLiteral);
-
                         return LoweredValInfo();
                     }
 
diff --git a/source/slang/slang-reflection.cpp b/source/slang/slang-reflection.cpp
index 306c45c14..8901f6bdd 100644
--- a/source/slang/slang-reflection.cpp
+++ b/source/slang/slang-reflection.cpp
@@ -1504,7 +1504,8 @@ SLANG_API SlangUInt spReflection_getHashedStringCount(
     SlangReflection*  reflection)
 {
     auto programLayout = convert(reflection);
-    return programLayout->hashedStringLiteralPool.getNumSlices() - StringSlicePool::kNumDefaultHandles;
+    auto slices = programLayout->hashedStringLiteralPool.getAdded();
+    return slices.getCount();
 }
 
 SLANG_API const char* spReflection_getHashedString(
@@ -1513,12 +1514,15 @@ SLANG_API const char* spReflection_getHashedString(
     size_t* outCount)
 {
     auto programLayout = convert(reflection);
-    UnownedStringSlice slice = programLayout->hashedStringLiteralPool.getSlice(StringSlicePool::Handle(index + StringSlicePool::kNumDefaultHandles));
+
+    auto slices = programLayout->hashedStringLiteralPool.getAdded();
+    auto slice = slices[Index(index)];
+
     *outCount = slice.size();
     return slice.begin();
 }
 
-SLANG_API int spCalcStringHash(const char* chars, size_t count)
+SLANG_API int spComputeStringHash(const char* chars, size_t count)
 {
     UnownedStringSlice slice(chars, count);
     return GetHashCode(slice);
diff --git a/source/slang/slang-source-loc.h b/source/slang/slang-source-loc.h
index 45b94bb5b..db2dcc394 100644
--- a/source/slang/slang-source-loc.h
+++ b/source/slang/slang-source-loc.h
@@ -376,7 +376,8 @@ struct SourceManager
     const List<SourceFile*>& getSourceFiles() const { return m_sourceFiles; }
 
     SourceManager() :
-        m_memoryArena(2048)
+        m_memoryArena(2048),
+        m_slicePool(StringSlicePool::Style::Default)
     {}
     ~SourceManager();
 
diff --git a/source/slang/slang-type-layout.h b/source/slang/slang-type-layout.h
index 1d3892d78..afca06ce1 100644
--- a/source/slang/slang-type-layout.h
+++ b/source/slang/slang-type-layout.h
@@ -732,6 +732,10 @@ public:
     TargetRequest* getTargetReq() { return targetProgram->getTargetReq(); }
     ComponentType* getProgram() { return targetProgram->getProgram(); }
 
+    ProgramLayout():
+        hashedStringLiteralPool(StringSlicePool::Style::Empty)
+    {
+    }
 
     // We catalog the requested entry points here,
     // and any entry-point-specific parameter data