Improvements around StringBlob (#2921)

* #include an absolute path didn't work - because paths were taken to always be relative.

* Small fixes and improvements around reflection tool.

* Make PrettyWriter printing a class.

* Improvements around handling StringBlob and storing stdlib source in ISlangBlob.

* Fix some issues with comments around StringBlob.

* Default initialize StringBlob fields.

6 files changed, 177 insertions, 100 deletions

diff --git a/source/core/slang-blob.cpp b/source/core/slang-blob.cpp
index a4acb98cb..c360f483b 100644
--- a/source/core/slang-blob.cpp
+++ b/source/core/slang-blob.cpp
@@ -35,25 +35,115 @@ void* BlobBase::castAs(const SlangUUID& guid)
 
 /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! StringBlob !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
 
-StringBlob::StringBlob(MoveUnique, String& in)
+void StringBlob::_uniqueInit(StringRepresentation* uniqueRep)
 {
-    auto rep = in.getStringRepresentation();
-    if (rep && !rep->isUniquelyReferenced())
+    // When initing the rep either has to be nullptr *or* uniquely referenced
+    // so we can take ownership of it.
+
+    SLANG_ASSERT(uniqueRep == nullptr || uniqueRep->isUniquelyReferenced());
+
+    m_rep = uniqueRep;
+
+    // If it's nullptr, that means the string is the empty string. To handle we 
+    // 
+    if (!uniqueRep)
+    {
+        m_chars = "";
+        m_charsCount = 0;
+    }
+    else
     {
-        // Make a new unique copy
-        m_string = in.getUnownedSlice();
+        m_chars = uniqueRep->getData();
+        m_charsCount = uniqueRep->getLength();
+    }
+
+    _checkRep();
+}
 
-        // Move out of in
-        String tmp;
-        tmp.swapWith(in);
+void StringBlob::_init(StringRepresentation* rep)
+{
+    if (rep)
+    {
+        // Even if the uniqueRep is 1, we can't assume we can take ownership
+        // So we make a copy
+
+        // If the length is 0, we can just init as empty
+        auto length = rep->getLength();
+        if (length == 0)
+        {
+            rep = nullptr;
+        }
+        else
+        {
+            const UnownedStringSlice slice(rep->getData(), length);
+            rep = StringRepresentation::createWithReference(slice);
+        }
+    }
+
+    // Must be unique at this point
+    _uniqueInit(rep);
+
+    _checkRep();
+}
+
+void StringBlob::_moveInit(StringRepresentation* rep)
+{
+    if (rep && !rep->isUniquelyReferenced())
+    {
+        // Will make a copy of the rep
+        _init(rep);
+        // We need to release a ref as rep is passed in with the 'current' ref count 
+        rep->releaseReference();
     }
     else
     {
-        // Must either not have a rep or be unique
-        m_string.swapWith(in);
+        _uniqueInit(rep);
+    }
+    _checkRep();
+}
+
+/* static */ComPtr<ISlangBlob> StringBlob::create(const UnownedStringSlice& slice)
+{
+    StringRepresentation* rep = nullptr;
+    if (slice.getLength())
+    {
+        rep = StringRepresentation::createWithReference(slice);
     }
+
+    // rep must be unique at this point
+    auto blob = new StringBlob;
+    blob->_uniqueInit(rep);
+    return ComPtr<ISlangBlob>(blob);
+}
+
+/* static */ComPtr<ISlangBlob> StringBlob::create(const String& in)
+{
+    auto blob = new StringBlob;
+    blob->_init(in.getStringRepresentation());
+    return ComPtr<ISlangBlob>(blob);
 }
 
+/* static */ComPtr<ISlangBlob> StringBlob::moveCreate(String& in)
+{
+    auto blob = new StringBlob;
+    blob->_moveInit(in.detachStringRepresentation());
+    return ComPtr<ISlangBlob>(blob);
+}
+
+/* static */ComPtr<ISlangBlob> StringBlob::moveCreate(String&& in)
+{
+    auto blob = new StringBlob;
+    blob->_moveInit(in.detachStringRepresentation());
+    return ComPtr<ISlangBlob>(blob);
+}
+
+StringBlob::~StringBlob()
+{
+    if (m_rep)
+    {
+        delete m_rep;
+    }
+}
 void* StringBlob::castAs(const SlangUUID& guid)
 {
     if (auto intf = getInterface(guid))
@@ -73,21 +163,11 @@ void* StringBlob::getObject(const Guid& guid)
     // Can always be accessed as terminated char*
     if (guid == SlangTerminatedChars::getTypeGuid())
     {
-        return const_cast<char*>(m_string.getBuffer());
+        return const_cast<char*>(m_chars);
     }
     return nullptr;
 }
 
-/* static */ComPtr<ISlangBlob> StringBlob::moveCreate(String& in)
-{
-    return ComPtr<ISlangBlob>(new StringBlob(MoveUnique{}, in));
-}
-
-/* static */ComPtr<ISlangBlob> StringBlob::moveCreate(String&& in)
-{
-    return ComPtr<ISlangBlob>(new StringBlob(MoveUnique{}, in));
-}
-
 /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! RawBlob !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
 
 void* RawBlob::castAs(const SlangUUID& guid)
diff --git a/source/core/slang-blob.h b/source/core/slang-blob.h
index aa503a6e5..f439a4eae 100644
--- a/source/core/slang-blob.h
+++ b/source/core/slang-blob.h
@@ -31,11 +31,10 @@ protected:
     void* getObject(const Guid& guid);
 };
 
-/** A blob that uses a `String` for its storage.
-NOTE! Returns length *WITHOUT* terminating 0, even though there is one.
+/** A blob that uses a `StringRepresentation` for its storage.
 
-NOTE! Whilst BobBase is atomic ref counted, the contained string *is not*. 
-There is a reasonable argument that StringBlob should contain it's own copy of the string contents.
+By design the StringBlob owns a unique reference to the StringRepresentation. 
+This is because StringBlob, implements an interface which should work across threads.
 */
 class StringBlob : public BlobBase
 {
@@ -46,37 +45,39 @@ public:
     virtual SLANG_NO_THROW void* SLANG_MCALL castAs(const SlangUUID& guid) SLANG_OVERRIDE;
 
     // ISlangBlob
-    SLANG_NO_THROW void const* SLANG_MCALL getBufferPointer() SLANG_OVERRIDE { return m_string.getBuffer(); }
-    SLANG_NO_THROW size_t SLANG_MCALL getBufferSize() SLANG_OVERRIDE { return m_string.getLength(); }
+    SLANG_NO_THROW void const* SLANG_MCALL getBufferPointer() SLANG_OVERRIDE { return m_chars; }
+    SLANG_NO_THROW size_t SLANG_MCALL getBufferSize() SLANG_OVERRIDE { return m_charsCount; }
 
-    static ComPtr<ISlangBlob> create(const String& in) { return ComPtr<ISlangBlob>(new StringBlob(in)); }
+        /// Since in is not being moved will *always* create a new representation, unless the in is empty
+    static ComPtr<ISlangBlob> create(const String& in);
+        /// Create from a slice
+    static ComPtr<ISlangBlob> create(const UnownedStringSlice& slice);
 
         /// Moves from in into the created blob. 
         /// NOTE! That will only use the representation from in, if it is *unique*
         /// otherwise it will make a new copy.
-        /// This is so that StringBlob won't hold a reference count via a string held externally.
-        /// In contrast StringBlob::create *may* share the representation.
     static ComPtr<ISlangBlob> moveCreate(String& in);
     static ComPtr<ISlangBlob> moveCreate(String&& in);
 
-protected:
-        /// A type that is only used to differentiate a constructor. Can construct with 
-        /// MoveUnique{}
-    struct MoveUnique {};
-
-    explicit StringBlob(String const& string)
-        : m_string(string)
-    {}
+        /// Dtor
+    ~StringBlob();
 
-    StringBlob(MoveUnique, String& string);
-    StringBlob() {}
+protected:
+        /// Use when rep *must* be unique (ie nullptr, and has a ref count of 1, that can be `taken` by the blob
+    void _uniqueInit(StringRepresentation* uniqueRep);
+        /// Init with a rep when can't be owned.
+    void _init(StringRepresentation* rep);
+        /// Init with a representation that has been moved.
+    void _moveInit(StringRepresentation* rep);
 
-        /// Get the contained string
-    SLANG_FORCE_INLINE const String& getString() const { return m_string; }
+        /// Checks that m_rep is either nullptr or has a ref count of 1 (ie it is owned by the blob)
+    SLANG_FORCE_INLINE void _checkRep() const { SLANG_ASSERT(m_rep == nullptr || m_rep->isUniquelyReferenced()); }
 
     void* getObject(const Guid& guid);
 
-    String m_string;
+    char* m_chars = nullptr;                ///< Pointer to the contained data. 
+    size_t m_charsCount = 0;                ///< The amount of chars *not* including terminating 0
+    StringRepresentation* m_rep = nullptr;  ///< Holds actual bytes. Can be nullptr if it's an empty string. 
 };
 
 class ListBlob : public BlobBase
diff --git a/source/core/slang-string.h b/source/core/slang-string.h
index 429a60c46..b4583d0b2 100644
--- a/source/core/slang-string.h
+++ b/source/core/slang-string.h
@@ -474,8 +474,12 @@ namespace Slang
             /// Append data written to buffer output via 'prepareForAppend' directly written 'inplace'
         void appendInPlace(const char* chars, Index count);
 
+            /// Get the internal string represenation
         SLANG_FORCE_INLINE StringRepresentation* getStringRepresentation() const { return m_buffer; }
 
+            /// Detach the representation (will leave string as empty). Rep ref count will remain unchanged.
+        SLANG_FORCE_INLINE StringRepresentation* detachStringRepresentation() { return m_buffer.detach(); }
+
         const char* begin() const
         {
             return getData();
diff --git a/source/slang/slang-compiler.h b/source/slang/slang-compiler.h
index 66454b2da..4e396e1f5 100755
--- a/source/slang/slang-compiler.h
+++ b/source/slang/slang-compiler.h
@@ -2988,18 +2988,19 @@ namespace Slang
 
         // Generated code for stdlib, etc.
         String stdlibPath;
-        String coreLibraryCode;
-        String slangLibraryCode;
-        String hlslLibraryCode;
-        String glslLibraryCode;
-        String autodiffLibraryCode;
-
-        String getStdlibPath();
-        String getCoreLibraryCode();
-        String getHLSLLibraryCode();
-        String getAutodiffLibraryCode();
-
-     
+
+        ComPtr<ISlangBlob> coreLibraryCode;
+        //ComPtr<ISlangBlob> slangLibraryCode;
+        ComPtr<ISlangBlob> hlslLibraryCode;
+        //ComPtr<ISlangBlob> glslLibraryCode;
+        ComPtr<ISlangBlob> autodiffLibraryCode;
+
+        String  getStdlibPath();
+
+        ComPtr<ISlangBlob> getCoreLibraryCode();
+        ComPtr<ISlangBlob> getHLSLLibraryCode();
+        ComPtr<ISlangBlob> getAutodiffLibraryCode();
+
         RefPtr<SharedASTBuilder> m_sharedASTBuilder;
 
 
diff --git a/source/slang/slang-stdlib.cpp b/source/slang/slang-stdlib.cpp
index a31c77985..3ef510990 100644
--- a/source/slang/slang-stdlib.cpp
+++ b/source/slang/slang-stdlib.cpp
@@ -14,16 +14,16 @@ namespace Slang
 {
     String Session::getStdlibPath()
     {
-        if(stdlibPath.getLength() != 0)
-            return stdlibPath;
-
-        // Make sure we have a line of text from __FILE__, that we'll extract the filename from
-        List<UnownedStringSlice> lines;
-        StringUtil::calcLines(UnownedStringSlice::fromLiteral(__FILE__), lines);
-        SLANG_ASSERT(lines.getCount() > 0 && lines[0].getLength() > 0);
+        if(stdlibPath.getLength() == 0)
+        {
+            // Make sure we have a line of text from __FILE__, that we'll extract the filename from
+            List<UnownedStringSlice> lines;
+            StringUtil::calcLines(UnownedStringSlice::fromLiteral(__FILE__), lines);
+            SLANG_ASSERT(lines.getCount() > 0 && lines[0].getLength() > 0);
 
-        // Make the path just the filename to remove issues around path being included on different targets
-        stdlibPath = Path::getFileName(lines[0]);
+            // Make the path just the filename to remove issues around path being included on different targets
+            stdlibPath = Path::getFileName(lines[0]);
+        }
         return stdlibPath;
     }
 
@@ -263,54 +263,45 @@ namespace Slang
 
 #define EMIT_LINE_DIRECTIVE() sb << "#line " << (__LINE__+1) << " \"" << path << "\"\n"
 
-    String Session::getCoreLibraryCode()
+    ComPtr<ISlangBlob> Session::getCoreLibraryCode()
     {
 #if !defined(SLANG_DISABLE_STDLIB_SOURCE)
-        if (coreLibraryCode.getLength() > 0)
-            return coreLibraryCode;
-
-        StringBuilder sb;
-
-        const String path = getStdlibPath();
-
-        #include "core.meta.slang.h"
-
-        coreLibraryCode = sb.produceString();
+        if (!coreLibraryCode)
+        {
+            StringBuilder sb;
+            const String path = getStdlibPath();
+            #include "core.meta.slang.h"
+            coreLibraryCode = StringBlob::moveCreate(sb);
+        }
 #endif
 
         return coreLibraryCode;
     }
 
-    String Session::getHLSLLibraryCode()
+    ComPtr<ISlangBlob> Session::getHLSLLibraryCode()
     {
 #if !defined(SLANG_DISABLE_STDLIB_SOURCE)
-        if (hlslLibraryCode.getLength() > 0)
-            return hlslLibraryCode;
-
-        const String path = getStdlibPath();
-
-        StringBuilder sb;
-
-        #include "hlsl.meta.slang.h"
-
-        hlslLibraryCode = sb.produceString();
+        if (!hlslLibraryCode)
+        {
+            const String path = getStdlibPath();
+            StringBuilder sb;
+            #include "hlsl.meta.slang.h"
+            hlslLibraryCode = StringBlob::moveCreate(sb);
+        }
 #endif
         return hlslLibraryCode;
     }
 
-    String Session::getAutodiffLibraryCode()
+    ComPtr<ISlangBlob> Session::getAutodiffLibraryCode()
     {
 #if !defined(SLANG_DISABLE_STDLIB_SOURCE)
-        if (autodiffLibraryCode.getLength() > 0)
-            return autodiffLibraryCode;
-
-        const String path = getStdlibPath();
-
-        StringBuilder sb;
-
-        #include "diff.meta.slang.h"
-
-        autodiffLibraryCode = sb.produceString();
+        if (!autodiffLibraryCode)
+        {
+            const String path = getStdlibPath();
+            StringBuilder sb;
+            #include "diff.meta.slang.h"
+            autodiffLibraryCode = StringBlob::moveCreate(sb);
+        }
 #endif
         return autodiffLibraryCode;
     }
diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
index d651f0737..5e0064226 100644
--- a/source/slang/slang.cpp
+++ b/source/slang/slang.cpp
@@ -313,9 +313,9 @@ SlangResult Session::compileStdLib(slang::CompileStdLibFlags compileFlags)
     }
 
     // TODO(JS): Could make this return a SlangResult as opposed to exception
-    addBuiltinSource(coreLanguageScope, "core", StringBlob::moveCreate(getCoreLibraryCode()));
-    addBuiltinSource(hlslLanguageScope, "hlsl", StringBlob::moveCreate(getHLSLLibraryCode()));
-    addBuiltinSource(autodiffLanguageScope, "diff", StringBlob::moveCreate(getAutodiffLibraryCode()));
+    addBuiltinSource(coreLanguageScope, "core", getCoreLibraryCode());
+    addBuiltinSource(hlslLanguageScope, "hlsl", getHLSLLibraryCode());
+    addBuiltinSource(autodiffLanguageScope, "diff", getAutodiffLibraryCode());
 
     if (compileFlags & slang::CompileStdLibFlag::WriteDocumentation)
     {