Serialization fixes based on review of #1547 (#1551)

* Test if blob is returned.

* Rename serialize files so can be grouped.

* StringRepresentationCache -> SerialStringTable

* Split out SerialStringTable from slang-serialize-ir

* First pass at reorganizing serialization/containers. Remain some issues about debug info.

* Fix bug in calculating sourceloc.

* Improve calcFixSourceLoc

* Make allocations for payload RiffContainer align to at least 8 bytes. This is important for read, if the payload can contain 8 byte aligned data. Note this has no effect on Riff file format alignment rules.

* Improve comments around RiffContainer and alignment.

* Remove SerialStringTable, can just use StringSlicePool instead.

* Add flags to control what is output in SerialContainer.
Turn off AST output for obfuscated code.
Lazily create astClasses when doing write container serialization.

* Typo fix for Clang/Linux.

* Fixes that came out of review

* TranslationUnit -> Module
* TargetModule -> TargetComponent
* PAYLOAD_MIN_ALIGNMENT -> kPayloadMinAlignment

6 files changed, 52 insertions, 49 deletions

diff --git a/source/core/slang-riff.cpp b/source/core/slang-riff.cpp
index 2162c40be..d64cd89c7 100644
--- a/source/core/slang-riff.cpp
+++ b/source/core/slang-riff.cpp
@@ -864,7 +864,7 @@ void RiffContainer::setPayload(Data* data, const void* payload, size_t size)
     data->m_ownership = Ownership::Arena;
     data->m_size = size;
 
-    data->m_payload = m_arena.allocateAligned(size, PAYLOAD_MIN_ALIGNMENT);
+    data->m_payload = m_arena.allocateAligned(size, kPayloadMinAlignment);
 
     if (payload)
     {
@@ -941,7 +941,7 @@ RiffContainer::Data* RiffContainer::makeSingleData(DataChunk* dataChunk)
         // Okay lets combine all into one block
         const size_t payloadSize = dataChunk->calcPayloadSize();
 
-        void* dst = m_arena.allocateAligned(payloadSize, PAYLOAD_MIN_ALIGNMENT);
+        void* dst = m_arena.allocateAligned(payloadSize, kPayloadMinAlignment);
         dataChunk->getPayload(dst);
 
         // Remove other datas
diff --git a/source/core/slang-riff.h b/source/core/slang-riff.h
index 43c5e285d..5a753b6d6 100644
--- a/source/core/slang-riff.h
+++ b/source/core/slang-riff.h
@@ -177,14 +177,11 @@ need to be recalculated, before serialization.
 class RiffContainer
 {
 public:
-    enum 
-    {
         // This alignment is only made for arena based allocations.
         // For external blocks it's client code to have appropriate alignment.
         // This is needed because when reading a RiffContainer, all allocation is arena based, and
         // if the payload contains 8 byte aligned data, the overall payload needs to be 8 byte aligned.
-        PAYLOAD_MIN_ALIGNMENT = 8,
-    };
+    static const size_t kPayloadMinAlignment = 8;
 
     enum class Ownership
     {
diff --git a/source/slang/slang-serialize-container.cpp b/source/slang/slang-serialize-container.cpp
index 23aacc5d3..2301da9d4 100644
--- a/source/slang/slang-serialize-container.cpp
+++ b/source/slang/slang-serialize-container.cpp
@@ -33,11 +33,15 @@ namespace Slang {
 
         SLANG_ASSERT(irModule || moduleDecl);
 
-        SerialContainerData::TranslationUnit dstTranslationUnit;
-        dstTranslationUnit.astRootNode = moduleDecl;
-        dstTranslationUnit.irModule = irModule;
+        SerialContainerData::Module dstModule;
 
-        out.translationUnits.add(dstTranslationUnit);
+        // NOTE: The astBuilder is not set here, as not needed to be scoped for serialization (it is assumed the
+        // TranslationUnitRequest stays in scope)
+
+        dstModule.astRootNode = moduleDecl;
+        dstModule.irModule = irModule;
+
+        out.modules.add(dstModule);
     }
 
     auto program = request->getSpecializedGlobalAndEntryPointsComponentType();
@@ -50,18 +54,18 @@ namespace Slang {
             auto targetProgram = program->getTargetProgram(target);
             auto irModule = targetProgram->getOrCreateIRModuleForLayout(sink);
 
-            SerialContainerData::TargetModule targetModule;
+            SerialContainerData::TargetComponent targetComponent;
 
-            targetModule.irModule = irModule;
+            targetComponent.irModule = irModule;
 
-            auto& dstTarget = targetModule.target;
+            auto& dstTarget = targetComponent.target;
 
             dstTarget.floatingPointMode = target->floatingPointMode;
             dstTarget.profile = target->targetProfile;
             dstTarget.flags = target->targetFlags;
             dstTarget.codeGenTarget = target->target;
 
-            out.targetModules.add(targetModule);
+            out.targetComponents.add(targetComponent);
         }
     }
 
@@ -105,10 +109,10 @@ namespace Slang {
         container->write(&containerHeader, sizeof(containerHeader));
     }
 
-    if (data.translationUnits.getCount() && (options.optionFlags & (SerialOptionFlag::IRModule | SerialOptionFlag::ASTModule)))
+    if (data.modules.getCount() && (options.optionFlags & (SerialOptionFlag::IRModule | SerialOptionFlag::ASTModule)))
     {
         // Module list
-        RiffContainer::ScopeChunk moduleListScope(container, RiffContainer::Chunk::Kind::List, SerialBinary::kTranslationUnitListFourCc);
+        RiffContainer::ScopeChunk moduleListScope(container, RiffContainer::Chunk::Kind::List, SerialBinary::kModuleListFourCc);
 
         if (options.optionFlags & SerialOptionFlag::DebugInfo)
         {
@@ -117,17 +121,17 @@ namespace Slang {
 
         RefPtr<ASTSerialClasses> astClasses;
 
-        for (const auto& translationUnit : data.translationUnits)
+        for (const auto& module : data.modules)
         {
             // Okay, we need to serialize this module to our container file.
             // We currently don't serialize it's name..., but support for that could be added.
 
             // Write the IR information
-            if ((options.optionFlags & SerialOptionFlag::IRModule) && translationUnit.irModule)
+            if ((options.optionFlags & SerialOptionFlag::IRModule) && module.irModule)
             {
                 IRSerialData serialData;
                 IRSerialWriter writer;
-                SLANG_RETURN_ON_FAIL(writer.write(translationUnit.irModule, debugWriter, options.optionFlags, &serialData));
+                SLANG_RETURN_ON_FAIL(writer.write(module.irModule, debugWriter, options.optionFlags, &serialData));
                 SLANG_RETURN_ON_FAIL(IRSerialWriter::writeContainer(serialData, options.compressionType, container));
             }
 
@@ -135,7 +139,7 @@ namespace Slang {
 
             if (options.optionFlags & SerialOptionFlag::ASTModule)
             {
-                if (ModuleDecl* moduleDecl = as<ModuleDecl>(translationUnit.astRootNode))
+                if (ModuleDecl* moduleDecl = as<ModuleDecl>(module.astRootNode))
                 {
                     if (!astClasses)
                     {
@@ -154,14 +158,14 @@ namespace Slang {
             }
         }
 
-        if (data.targetModules.getCount() && (options.optionFlags & SerialOptionFlag::IRModule))
+        if (data.targetComponents.getCount() && (options.optionFlags & SerialOptionFlag::IRModule))
         {
             // TODO: in the case where we have specialization, we might need
             // to serialize IR related to `program`...
 
-            for (const auto& targetModule : data.targetModules)
+            for (const auto& targetComponent : data.targetComponents)
             {
-                IRModule* irModule = targetModule.irModule;
+                IRModule* irModule = targetComponent.irModule;
 
                 // Okay, we need to serialize this target program and its IR too...
                 IRSerialData serialData;
@@ -255,10 +259,10 @@ namespace Slang {
         SLANG_RETURN_ON_FAIL(debugReader->read(&debugData, options.sourceManager));
     }
 
-    // Add translation units
-    if (RiffContainer::ListChunk* translationUnits = containerChunk->findContainedList(SerialBinary::kTranslationUnitListFourCc))
+    // Add modules
+    if (RiffContainer::ListChunk* moduleList = containerChunk->findContainedList(SerialBinary::kModuleListFourCc))
     {
-        RiffContainer::Chunk* chunk = translationUnits->getFirstContainedChunk();
+        RiffContainer::Chunk* chunk = moduleList->getFirstContainedChunk();
         while (chunk)
         {
             auto startChunk = chunk;
@@ -297,7 +301,7 @@ namespace Slang {
                     // For now we just generate a name.
 
                     StringBuilder buf;
-                    buf << "tu" << out.translationUnits.getCount();
+                    buf << "tu" << out.modules.getCount();
 
                     astBuilder = new ASTBuilder(options.sharedASTBuilder, buf.ProduceString());
 
@@ -315,13 +319,13 @@ namespace Slang {
 
             if (astBuilder || irModule)
             {
-                SerialContainerData::TranslationUnit translationUnit;
+                SerialContainerData::Module module;
 
-                translationUnit.astBuilder = astBuilder;
-                translationUnit.astRootNode = astRootNode;
-                translationUnit.irModule = irModule;
+                module.astBuilder = astBuilder;
+                module.astRootNode = astRootNode;
+                module.irModule = irModule;
 
-                out.translationUnits.add(translationUnit);
+                out.modules.add(module);
             }
 
             // If no progress, step to next chunk
diff --git a/source/slang/slang-serialize-container.h b/source/slang/slang-serialize-container.h
index 0bed80568..c34bebb6c 100644
--- a/source/slang/slang-serialize-container.h
+++ b/source/slang/slang-serialize-container.h
@@ -41,18 +41,18 @@ struct SerialContainerData
         FloatingPointMode   floatingPointMode = FloatingPointMode::Default;
     };
     
-    struct TargetModule
+    struct TargetComponent
     {
         // IR module for a specific compilation target
         Target target;
         RefPtr<IRModule> irModule;
     };
 
-    struct TranslationUnit
+    struct Module
     {
-        RefPtr<IRModule> irModule;
-        RefPtr<ASTBuilder> astBuilder;
-        NodeBase* astRootNode = nullptr;
+        RefPtr<IRModule> irModule;              ///< The IR for the module
+        RefPtr<ASTBuilder> astBuilder;          ///< The astBuilder that owns the astRootNode
+        NodeBase* astRootNode = nullptr;        ///< The module decl
     };
 
     struct EntryPoint
@@ -65,12 +65,12 @@ struct SerialContainerData
     void clear()
     {
         entryPoints.clear();
-        translationUnits.clear();
-        targetModules.clear();
+        modules.clear();
+        targetComponents.clear();
     }
 
-    List<TranslationUnit> translationUnits;
-    List<TargetModule> targetModules;
+    List<Module> modules;
+    List<TargetComponent> targetComponents;
     List<EntryPoint> entryPoints;
 };
 
@@ -78,9 +78,9 @@ struct SerialContainerUtil
 {
     struct WriteOptions
     {
-        SerialCompressionType compressionType = SerialCompressionType::VariableByteLite;
-        SerialOptionFlags optionFlags = SerialOptionFlag::ASTModule | SerialOptionFlag::IRModule;
-        SourceManager* sourceManager = nullptr;
+        SerialCompressionType compressionType = SerialCompressionType::VariableByteLite;                ///< If compression is used what type to use (only some parts can be compressed)
+        SerialOptionFlags optionFlags = SerialOptionFlag::ASTModule | SerialOptionFlag::IRModule;       ///< Flags controlling what is written
+        SourceManager* sourceManager = nullptr;                                                         ///< The source manager used for the SourceLoc in the input
     };
 
     struct ReadOptions
@@ -94,9 +94,11 @@ struct SerialContainerUtil
         /// Get the serializable contents of the request as data
     static SlangResult requestToData(EndToEndCompileRequest* request, const WriteOptions& options, SerialContainerData& outData);
 
+        /// Write the data into the container
     static SlangResult write(const SerialContainerData& data, const WriteOptions& options, RiffContainer* container);
 
-    static SlangResult read(RiffContainer* container, const ReadOptions& options, SerialContainerData& out);
+        /// Read the container into outData
+    static SlangResult read(RiffContainer* container, const ReadOptions& options, SerialContainerData& outData);
 
         /// Verify IR serialization
     static SlangResult verifyIRSerialize(IRModule* module, Session* session, const WriteOptions& options);
diff --git a/source/slang/slang-serialize-types.h b/source/slang/slang-serialize-types.h
index 405a7bfcd..b0e28efcb 100644
--- a/source/slang/slang-serialize-types.h
+++ b/source/slang/slang-serialize-types.h
@@ -127,7 +127,7 @@ struct SerialBinary
     static const FourCC kStringTableFourCc = SLANG_FOUR_CC('S', 'L', 's', 't');
 
         /// TranslationUnitList
-    static const FourCC kTranslationUnitListFourCc = SLANG_FOUR_CC('S', 'L', 'm', 'l');
+    static const FourCC kModuleListFourCc = SLANG_FOUR_CC('S', 'L', 'm', 'l');
 
         /// An entry point
     static const FourCC kEntryPointFourCc = SLANG_FOUR_CC('E', 'P', 'n', 't');
diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
index 43d1ede7e..fbdbe14b6 100644
--- a/source/slang/slang.cpp
+++ b/source/slang/slang.cpp
@@ -3039,12 +3039,12 @@ SlangResult _addLibraryReference(EndToEndCompileRequest* req, Stream* stream)
 
         SLANG_RETURN_ON_FAIL(SerialContainerUtil::read(&riffContainer, options, containerData));
 
-        for (const auto& translationUnit : containerData.translationUnits)
+        for (const auto& module : containerData.modules)
         {
             // If the irModule is set, add it
-            if (translationUnit.irModule)
+            if (module.irModule)
             {
-                linkage->m_libModules.add(translationUnit.irModule);
+                linkage->m_libModules.add(module.irModule);
             }
         }