IncludeFileSystem -> DefaultFileSystem (shader-slang#677)

Improvements in 'singleton'ness of DefaultFileSystem
Made WrapFileSystem a stand alone type - to remove 'odd' aspects of deriving from DefaultFileSystem (such as inheriting getSingleton method/fixing ref counting)
Simplified CompileRequest::loadFile - becauce fileSystemExt is always available.

Author: jsmall-zzz

slang-com-helper.h

@@ -98,7 +98,7 @@ SLANG_NO_THROW uint32_t SLANG_MCALL release() \
         return 0; \
     } \
     return m_refCount; \
-} \
+} 
 
 #define SLANG_IUNKNOWN_ALL \
     SLANG_IUNKNOWN_QUERY_INTERFACE \

source/slang/include-file-system.cpp source/slang/default-file-system.cpp

@@ -1,4 +1,4 @@
-#include "include-file-system.h"
+#include "default-file-system.h"
 
 #include "../../slang-com-ptr.h"
 #include "../core/slang-io.h"
@@ -14,19 +14,14 @@ static const Guid IID_ISlangFileSystemExt = SLANG_UUID_ISlangFileSystemExt;
 
 /* !!!!!!!!!!!!!!!!!!!!!!!!!! IncludeFileSystem !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!*/
 
-/* static */ISlangFileSystemExt* IncludeFileSystem::getDefault()
-{
-    static IncludeFileSystem s_includeFileSystem;
-    s_includeFileSystem.ensureRef();
-    return &s_includeFileSystem;
-}
+/* static */DefaultFileSystem DefaultFileSystem::s_singleton; 
 
-ISlangUnknown* IncludeFileSystem::getInterface(const Guid& guid)
+ISlangUnknown* DefaultFileSystem::getInterface(const Guid& guid)
 {
     return (guid == IID_ISlangUnknown || guid == IID_ISlangFileSystem || guid == IID_ISlangFileSystemExt) ? static_cast<ISlangFileSystemExt*>(this) : nullptr;
 }
 
-SlangResult IncludeFileSystem::getCanoncialPath(const char* path, ISlangBlob** canonicalPathOut)
+SlangResult DefaultFileSystem::getCanoncialPath(const char* path, ISlangBlob** canonicalPathOut)
 {
     String canonicalPath;
     SLANG_RETURN_ON_FAIL(Path::GetCanonical(path, canonicalPath));
@@ -35,7 +30,7 @@ SlangResult IncludeFileSystem::getCanoncialPath(const char* path, ISlangBlob** c
     return SLANG_OK;
 }
 
-SlangResult IncludeFileSystem::calcRelativePath(SlangPathType fromPathType, const char* fromPath, const char* path, ISlangBlob** pathOut)
+SlangResult DefaultFileSystem::calcRelativePath(SlangPathType fromPathType, const char* fromPath, const char* path, ISlangBlob** pathOut)
 {
     String relPath;
     switch (fromPathType)
@@ -56,15 +51,14 @@ SlangResult IncludeFileSystem::calcRelativePath(SlangPathType fromPathType, cons
     return SLANG_OK;
 }
 
-SlangResult SLANG_MCALL IncludeFileSystem::loadFile(char const* path, ISlangBlob** outBlob)
+SlangResult DefaultFileSystem::loadFile(char const* path, ISlangBlob** outBlob)
 {
-    // Otherwise, fall back to a default implementation that uses the `core`
+    // Default implementation that uses the `core`
     // libraries facilities for talking to the OS filesystem.
     //
     // TODO: we might want to conditionally compile these in, so that
     // a user could create a build of Slang that doesn't include any OS
     // filesystem calls.
-    //
 
     if (!File::Exists(path))
     {
@@ -86,16 +80,31 @@ SlangResult SLANG_MCALL IncludeFileSystem::loadFile(char const* path, ISlangBlob
 
 /* !!!!!!!!!!!!!!!!!!!!!!!!!! WrapFileSystem !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!*/
 
-SlangResult SLANG_MCALL WrapFileSystem::loadFile(char const* path, ISlangBlob** outBlob)
+ISlangUnknown* WrapFileSystem::getInterface(const Guid& guid)
+{
+    return (guid == IID_ISlangUnknown || guid == IID_ISlangFileSystem || guid == IID_ISlangFileSystemExt) ? static_cast<ISlangFileSystemExt*>(this) : nullptr;
+}
+
+SlangResult WrapFileSystem::loadFile(char const* path, ISlangBlob** outBlob)
 {
+    // Used the wrapped file system to do the loading
     return m_fileSystem->loadFile(path, outBlob);
 }
 
 SlangResult WrapFileSystem::getCanoncialPath(const char* path, ISlangBlob** canonicalPathOut)
 {
+    // This isn't a very good 'canonical  path' because the same file might be referenced 
+    // multiple ways - for example by using relative paths. 
+    // But it's simple and matches slangs previous behavior. 
     String canonicalPath(path);
     *canonicalPathOut = createStringBlob(canonicalPath).detach();
     return SLANG_OK;
 }
 
+SlangResult WrapFileSystem::calcRelativePath(SlangPathType fromPathType, const char* fromPath, const char* path, ISlangBlob** pathOut)
+{
+    // Just defer to the default implementation
+    return DefaultFileSystem::getSingleton()->calcRelativePath(fromPathType, fromPath, path, pathOut);
+}
+
 } 

source/slang/include-file-system.h source/slang/default-file-system.h

@@ -1,5 +1,5 @@
-#ifndef SLANG_INCLUDE_FILE_SYSTEM_H_INCLUDED
-#define SLANG_INCLUDE_FILE_SYSTEM_H_INCLUDED
+#ifndef SLANG_DEFAULT_FILE_SYSTEM_H_INCLUDED
+#define SLANG_DEFAULT_FILE_SYSTEM_H_INCLUDED
 
 #include "../../slang.h"
 #include "../../slang-com-helper.h"
@@ -8,12 +8,15 @@
 namespace Slang
 {
 
-class IncludeFileSystem : public ISlangFileSystemExt
+class DefaultFileSystem : public ISlangFileSystemExt
 {
 public:
-    // ISlangUnknown
-    SLANG_IUNKNOWN_ALL
-        
+    // ISlangUnknown 
+    // override ref counting, as DefaultFileSystem is singleton
+    SLANG_IUNKNOWN_QUERY_INTERFACE 
+    SLANG_NO_THROW uint32_t SLANG_MCALL addRef() SLANG_OVERRIDE { return 1; }
+    SLANG_NO_THROW uint32_t SLANG_MCALL release() SLANG_OVERRIDE { return 1; } 
+
     // ISlangFileSystem
     virtual SLANG_NO_THROW SlangResult SLANG_MCALL loadFile(
         char const*     path,
@@ -31,31 +34,31 @@ class IncludeFileSystem : public ISlangFileSystemExt
         ISlangBlob** pathOut) SLANG_OVERRIDE;
 
         /// Get a default instance
-    static ISlangFileSystemExt* getDefault();
+    static ISlangFileSystemExt* getSingleton() { return &s_singleton; }
 
-protected:
-  
-        /// If no ref, add one to the ref
-    void ensureRef() { m_refCount += (m_refCount == 0); }
+private:
+        /// Make so not constructable
+    DefaultFileSystem() {}
 
     ISlangUnknown* getInterface(const Guid& guid);
-    uint32_t m_refCount = 0;
+
+    static DefaultFileSystem s_singleton;
 };
 
 /* Wraps an ISlangFileSystem, and provides the extra methods required to make a ISlangFileSystemExt 
 interface, deferring to the contained file system to do reading. 
 
 NOTE! That this behavior is the same as previously in that.... 
-1) getRelativePath, just returns the path as processed by the Path:: methods 
+1) calcRelativePath, just returns the path as processed by the Path:: methods 
 2) getCanonicalPath, just returns the input path as the 'canonical' path. This will be wrong with a file multiply referenced through paths with .. and or . but 
 doing it this way means it works as before and requires no new functions.
 */
-class WrapFileSystem : public IncludeFileSystem
+class WrapFileSystem : public ISlangFileSystemExt
 {
 public:
-    // So we don't need virtual dtor
-    SLANG_IUNKNOWN_RELEASE
-
+    // ISlangUnknown 
+    SLANG_IUNKNOWN_ALL
+    
     // ISlangFileSystem
     virtual SLANG_NO_THROW SlangResult SLANG_MCALL loadFile(
         char const*     path,
@@ -66,14 +69,23 @@ class WrapFileSystem : public IncludeFileSystem
         const char* path,
         ISlangBlob** canonicalPathOut) SLANG_OVERRIDE;
 
+    virtual SLANG_NO_THROW SlangResult SLANG_MCALL calcRelativePath(
+        SlangPathType fromPathType,
+        const char* fromPath,
+        const char* path,
+        ISlangBlob** pathOut) SLANG_OVERRIDE;
+
         /// Ctor
     WrapFileSystem(ISlangFileSystem* fileSystem):
         m_fileSystem(fileSystem)
     {
     }
 
 protected:
+    ISlangUnknown* getInterface(const Guid& guid);
+
     ComPtr<ISlangFileSystem> m_fileSystem;                  ///< The wrapped file system
+    uint32_t m_refCount = 0;
 };
 
 }

source/slang/slang.cpp

@@ -9,7 +9,7 @@
 #include "syntax-visitors.h"
 #include "../slang/type-layout.h"
 
-#include "include-file-system.h"
+#include "default-file-system.h"
 
 #include "ir-serialize.h"
 
@@ -316,7 +316,7 @@ CompileRequest::CompileRequest(Session* session)
 
     // Set up the default file system
     SLANG_ASSERT(fileSystem == nullptr);
-    fileSystemExt = IncludeFileSystem::getDefault();
+    fileSystemExt = DefaultFileSystem::getSingleton();
 }
 
 CompileRequest::~CompileRequest()
@@ -407,42 +407,9 @@ ComPtr<ISlangBlob> createRawBlob(void const* inData, size_t size)
 
 SlangResult CompileRequest::loadFile(String const& path, ISlangBlob** outBlob)
 {
-    // If there is a used-defined filesystem, then use that to load files.
-    //
-    if(fileSystem)
-    {
-        return fileSystem->loadFile(path.Buffer(), outBlob);
-    }
-
-    // Otherwise, fall back to a default implementation that uses the `core`
-    // libraries facilities for talking to the OS filesystem.
-    //
-    // TODO: we might want to conditionally compile these in, so that
-    // a user could create a build of Slang that doesn't include any OS
-    // filesystem calls.
-    //
-
-    if (!File::Exists(path))
-    {
-        return SLANG_FAIL;
-    }
-
-    try
-    {
-        String sourceString = File::ReadAllText(path);
-        ComPtr<ISlangBlob> sourceBlob = createStringBlob(sourceString);
-        *outBlob = sourceBlob.detach();
-
-        return SLANG_OK;
-    }
-    catch(...)
-    {
-    }
-    return SLANG_FAIL;
-
+    return fileSystemExt->loadFile(path.Buffer(), outBlob);
 }
 
-
 RefPtr<Expr> CompileRequest::parseTypeString(TranslationUnitRequest * translationUnit, String typeStr, RefPtr<Scope> scope)
 {
     // Create a SourceManager on the stack, so any allocations for 'SourceFile'/'SourceView' etc will be cleaned up
@@ -1214,7 +1181,7 @@ SLANG_API void spSetFileSystem(
     // Set up fileSystemExt appropriately
     if (fileSystem == nullptr)
     {
-        req->fileSystemExt = Slang::IncludeFileSystem::getDefault();
+        req->fileSystemExt = Slang::DefaultFileSystem::getSingleton();
     }
     else
     {

source/slang/slang.vcxproj

@@ -175,13 +175,13 @@
     <ClInclude Include="compiler.h" />
     <ClInclude Include="core.meta.slang.h" />
     <ClInclude Include="decl-defs.h" />
+    <ClInclude Include="default-file-system.h" />
     <ClInclude Include="diagnostic-defs.h" />
     <ClInclude Include="diagnostics.h" />
     <ClInclude Include="emit.h" />
     <ClInclude Include="expr-defs.h" />
     <ClInclude Include="glsl.meta.slang.h" />
     <ClInclude Include="hlsl.meta.slang.h" />
-    <ClInclude Include="include-file-system.h" />
     <ClInclude Include="ir-constexpr.h" />
     <ClInclude Include="ir-dominators.h" />
     <ClInclude Include="ir-inst-defs.h" />
@@ -228,10 +228,10 @@
     <ClCompile Include="bytecode.cpp" />
     <ClCompile Include="check.cpp" />
     <ClCompile Include="compiler.cpp" />
+    <ClCompile Include="default-file-system.cpp" />
     <ClCompile Include="diagnostics.cpp" />
     <ClCompile Include="dxc-support.cpp" />
     <ClCompile Include="emit.cpp" />
-    <ClCompile Include="include-file-system.cpp" />
     <ClCompile Include="ir-constexpr.cpp" />
     <ClCompile Include="ir-dominators.cpp" />
     <ClCompile Include="ir-legalize-types.cpp" />

source/slang/slang.vcxproj.filters

@@ -24,6 +24,9 @@
     <ClInclude Include="decl-defs.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="default-file-system.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
     <ClInclude Include="diagnostic-defs.h">
       <Filter>Header Files</Filter>
     </ClInclude>
@@ -42,9 +45,6 @@
     <ClInclude Include="hlsl.meta.slang.h">
       <Filter>Header Files</Filter>
     </ClInclude>
-    <ClInclude Include="include-file-system.h">
-      <Filter>Header Files</Filter>
-    </ClInclude>
     <ClInclude Include="ir-constexpr.h">
       <Filter>Header Files</Filter>
     </ClInclude>
@@ -179,6 +179,9 @@
     <ClCompile Include="compiler.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="default-file-system.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
     <ClCompile Include="diagnostics.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
@@ -188,9 +191,6 @@
     <ClCompile Include="emit.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="include-file-system.cpp">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="ir-constexpr.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>