Improvements around file tracking and Artifacts (#2379)

8 files changed, 95 insertions, 128 deletions

diff --git a/source/compiler-core/slang-artifact-handler-impl.cpp b/source/compiler-core/slang-artifact-handler-impl.cpp
index b9dfb29b1..c8e7bf247 100644
--- a/source/compiler-core/slang-artifact-handler-impl.cpp
+++ b/source/compiler-core/slang-artifact-handler-impl.cpp
@@ -145,7 +145,7 @@ SlangResult DefaultArtifactHandler::getOrCreateRepresentation(IArtifact* artifac
 	if (guid == ISlangSharedLibrary::getTypeGuid())
 	{
 		ComPtr<ISlangSharedLibrary> sharedLib;
-		SLANG_RETURN_ON_FAIL(_loadSharedLibrary(artifact, keep, sharedLib.writeRef()));
+		SLANG_RETURN_ON_FAIL(_loadSharedLibrary(artifact, sharedLib.writeRef()));
 		return _addRepresentation(artifact, keep, sharedLib, outCastable);
 	}
 
@@ -223,7 +223,7 @@ SlangResult DefaultArtifactHandler::getOrCreateFileRepresentation(IArtifact* art
 	return SLANG_OK;
 }
 
-SlangResult DefaultArtifactHandler::_loadSharedLibrary(IArtifact* artifact, ArtifactKeep keep, ISlangSharedLibrary** outSharedLibrary)
+SlangResult DefaultArtifactHandler::_loadSharedLibrary(IArtifact* artifact, ISlangSharedLibrary** outSharedLibrary)
 {
 	// If it is 'shared library' for a CPU like thing, we can try and load it
 	const auto desc = artifact->getDesc();
@@ -233,7 +233,10 @@ SlangResult DefaultArtifactHandler::_loadSharedLibrary(IArtifact* artifact, Arti
 	{
 		// Get as a file represenation on the OS file system
 		ComPtr<IFileArtifactRepresentation> fileRep;
-		SLANG_RETURN_ON_FAIL(artifact->requireFile(ArtifactKeep::No, nullptr, fileRep.writeRef()));
+
+		// We want to keep the file representation, otherwise every request, could produce a new file
+		// and that seems like a bad idea.
+		SLANG_RETURN_ON_FAIL(artifact->requireFile(ArtifactKeep::Yes, nullptr, fileRep.writeRef()));
 
 		// We requested on the OS file system, just check that's what we got...
 		SLANG_ASSERT(fileRep->getFileSystem() == nullptr);
@@ -244,22 +247,8 @@ SlangResult DefaultArtifactHandler::_loadSharedLibrary(IArtifact* artifact, Arti
 		{
 			return SLANG_FAIL;
 		}
-
-		// The ScopeSharedLibrary will keep the fileRep in scope as long as is needed
-		auto sharedLibrary = new ScopeSharedLibrary(handle, fileRep);
-
-		if (canKeep(keep))
-		{
-			// We want to keep the fileRep, as that is necessary for the sharedLibrary to even work
-			artifact->addRepresentation(fileRep);
-			// Keep the shared library
-			artifact->addRepresentation(sharedLibrary);
-		}
-
 		// Output
-		sharedLibrary->addRef();
-		*outSharedLibrary = sharedLibrary;
-
+		*outSharedLibrary = ScopeSharedLibrary::create(handle, fileRep).detach();
 		return SLANG_OK;
 	}
 
diff --git a/source/compiler-core/slang-artifact-handler-impl.h b/source/compiler-core/slang-artifact-handler-impl.h
index f49fa4230..88be93f55 100644
--- a/source/compiler-core/slang-artifact-handler-impl.h
+++ b/source/compiler-core/slang-artifact-handler-impl.h
@@ -28,7 +28,7 @@ public:
 	static IArtifactHandler* getSingleton() { return &g_singleton; }
 protected:
 
-	SlangResult _loadSharedLibrary(IArtifact* artifact, ArtifactKeep keep, ISlangSharedLibrary** outSharedLibrary);
+	SlangResult _loadSharedLibrary(IArtifact* artifact, ISlangSharedLibrary** outSharedLibrary);
 	
 	void* getInterface(const Guid& uuid);
 	void* getObject(const Guid& uuid);
diff --git a/source/compiler-core/slang-artifact-impl.cpp b/source/compiler-core/slang-artifact-impl.cpp
index d8646eb20..056d89a4b 100644
--- a/source/compiler-core/slang-artifact-impl.cpp
+++ b/source/compiler-core/slang-artifact-impl.cpp
@@ -26,8 +26,7 @@ static bool _checkChildren(IArtifact::FindStyle findStyle)
 
 IArtifactHandler* Artifact::_getHandler()
 {
-    // TODO(JS): For now we just use the default handler, but in the future this should probably be a member
-    return DefaultArtifactHandler::getSingleton();
+    return m_handler ? m_handler : DefaultArtifactHandler::getSingleton();
 }
 
 void* Artifact::castAs(const Guid& guid)
@@ -98,6 +97,16 @@ SlangResult Artifact::loadSharedLibrary(ArtifactKeep keep, ISlangSharedLibrary**
     return SLANG_OK;
 }
 
+IArtifactHandler* Artifact::getHandler()
+{
+    return m_handler;
+}
+
+void Artifact::setHandler(IArtifactHandler* handler)
+{
+    m_handler = handler;
+}
+
 SlangResult Artifact::getOrCreateRepresentation(const Guid& typeGuid, ArtifactKeep keep, ICastable** outCastable)
 {
     auto handler = _getHandler();
diff --git a/source/compiler-core/slang-artifact-impl.h b/source/compiler-core/slang-artifact-impl.h
index c5c7307a1..9901b9d63 100644
--- a/source/compiler-core/slang-artifact-impl.h
+++ b/source/compiler-core/slang-artifact-impl.h
@@ -62,6 +62,9 @@ public:
     virtual SLANG_NO_THROW ICastable* SLANG_MCALL findRepresentationWithPredicate(ICastableList::FindFunc findFunc, void* data) SLANG_OVERRIDE;
     virtual SLANG_NO_THROW SlangResult SLANG_MCALL getOrCreateRepresentation(const Guid& typeGuid, ArtifactKeep keep, ICastable** outCastable) SLANG_OVERRIDE;
 
+    virtual SLANG_NO_THROW IArtifactHandler* SLANG_MCALL getHandler() SLANG_OVERRIDE;
+    virtual SLANG_NO_THROW void SLANG_MCALL setHandler(IArtifactHandler* handler) SLANG_OVERRIDE;
+
     virtual SLANG_NO_THROW Slice<IArtifact*> SLANG_MCALL getChildren() SLANG_OVERRIDE { return Slice<IArtifact*>(nullptr, 0); }
 
     virtual SLANG_NO_THROW IArtifact* SLANG_MCALL findArtifactByDerivedDesc(FindStyle findStyle, const ArtifactDesc& desc) SLANG_OVERRIDE;
@@ -103,6 +106,8 @@ protected:
 
     String m_name;                              ///< Name of this artifact
 
+    ComPtr<IArtifactHandler> m_handler;         ///< The handler. Can be nullptr and then default handler is used.
+
     LazyCastableList m_associated;              ///< Associated items
     LazyCastableList m_representations;         ///< Representations
 };
diff --git a/source/compiler-core/slang-artifact.h b/source/compiler-core/slang-artifact.h
index 1fae7ab8a..da9934388 100644
--- a/source/compiler-core/slang-artifact.h
+++ b/source/compiler-core/slang-artifact.h
@@ -395,7 +395,6 @@ public:
     virtual SLANG_NO_THROW void* SLANG_MCALL findRepresentation(const Guid& guid) = 0;
         /// Find first representation that matches the predicate 
     virtual SLANG_NO_THROW ICastable* SLANG_MCALL findRepresentationWithPredicate(ICastableList::FindFunc findFunc, void* data) = 0;
-
         /// Get all the representations
     virtual SLANG_NO_THROW Slice<ICastable*> SLANG_MCALL getRepresentations() = 0;
         /// Get the list of all representations
@@ -405,6 +404,11 @@ public:
         /// If found outCastable holds an entity that *must* be castable to typeGuid
     virtual SLANG_NO_THROW SlangResult SLANG_MCALL getOrCreateRepresentation(const Guid& typeGuid, ArtifactKeep keep, ICastable** outCastable) = 0;
     
+        /// Get the handler used for this artifact. If nullptr means the default handler will be used.
+    virtual SLANG_NO_THROW IArtifactHandler* SLANG_MCALL getHandler() = 0;
+        /// Set the handler associated with this artifact. Setting nullptr will use the default handler.
+    virtual SLANG_NO_THROW void SLANG_MCALL setHandler(IArtifactHandler* handler) = 0;
+
         /// Get the children, will only remain valid if no mutation of children list
     virtual SLANG_NO_THROW Slice<IArtifact*> SLANG_MCALL getChildren() = 0;
 
diff --git a/source/compiler-core/slang-downstream-compiler.cpp b/source/compiler-core/slang-downstream-compiler.cpp
index 13d605b3d..aa3a44975 100644
--- a/source/compiler-core/slang-downstream-compiler.cpp
+++ b/source/compiler-core/slang-downstream-compiler.cpp
@@ -64,74 +64,6 @@ void* DownstreamCompilerBase::getObject(const Guid& guid)
     return nullptr;
 }
 
-/* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! CommandLineDownstreamArtifactRepresentation !!!!!!!!!!!!!!!!!!!!!!*/
-
-void* CommandLineDownstreamArtifactRepresentation::castAs(const Guid& guid)
-{
-    if (auto ptr = getInterface(guid))
-    {
-        return ptr;
-    }
-    return getObject(guid);
-}
-
-void* CommandLineDownstreamArtifactRepresentation::getInterface(const Guid& guid)
-{
-    if (guid == ISlangUnknown::getTypeGuid() ||
-        guid == ICastable::getTypeGuid() ||
-        guid == IArtifactRepresentation::getTypeGuid())
-    {
-        IArtifactRepresentation* rep = this;
-        return rep;
-    }
-
-    return nullptr;
-}
-
-void* CommandLineDownstreamArtifactRepresentation::getObject(const Guid& guid)
-{
-    SLANG_UNUSED(guid);
-    return nullptr;
-}
-
-SlangResult CommandLineDownstreamArtifactRepresentation::createRepresentation(const Guid& typeGuid, ICastable** outCastable)
-{
-    if (typeGuid == ISlangSharedLibrary::getTypeGuid())
-    {
-        // Okay we want to load
-        // Try loading the shared library
-        SharedLibrary::Handle handle;
-        if (SLANG_FAILED(SharedLibrary::loadWithPlatformPath(m_moduleFilePath.getBuffer(), handle)))
-        {
-            return SLANG_FAIL;
-        }
-
-        // The shared library needs to keep temp files in scope
-        ComPtr<ISlangSharedLibrary> lib(new ScopeSharedLibrary(handle, m_artifactList));
-        *outCastable = lib.detach();
-        return SLANG_OK;
-    }
-    else if (typeGuid == ISlangBlob::getTypeGuid())
-    {
-        List<uint8_t> contents;
-        // Read the binary
-            // Read the contents of the binary
-        SLANG_RETURN_ON_FAIL(File::readAllBytes(m_moduleFilePath, contents));
-
-        auto blob = ScopeBlob::create(ListBlob::moveCreate(contents), m_artifactList);
-
-        *outCastable = CastableUtil::getCastable(blob).detach();
-        return SLANG_OK;
-    }
-
-    return SLANG_E_NOT_AVAILABLE;
-}
-
-bool CommandLineDownstreamArtifactRepresentation::exists()
-{
-    return File::exists(m_moduleFilePath);
-}
-
 /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! CommandLineDownstreamCompiler !!!!!!!!!!!!!!!!!!!!!!*/
 
 static bool _isContentsInFile(const DownstreamCompileOptions& options)
@@ -238,25 +170,33 @@ SlangResult CommandLineDownstreamCompiler::compile(const CompileOptions& inOptio
     // Append command line args to the end of cmdLine using the target specific function for the specified options
     SLANG_RETURN_ON_FAIL(calcArgs(options, cmdLine));
 
-    String moduleFilePath;
-
+    // The 'mainArtifact' is the main product produced from the compilation - the executable/sharedlibrary/object etc
+    ComPtr<IArtifact> mainArtifact;
     {
-        StringBuilder builder;
         const auto desc = ArtifactDescUtil::makeDescForCompileTarget(options.targetType);
-        SLANG_RETURN_ON_FAIL(ArtifactDescUtil::calcPathForDesc(desc, options.modulePath.getUnownedSlice(), builder));
-
-        moduleFilePath = builder.ProduceString();
-    }
 
-    {
         List<ComPtr<IArtifact>> artifacts;
         SLANG_RETURN_ON_FAIL(calcCompileProducts(options, DownstreamProductFlag::All, lockFile, artifacts));
 
         for (IArtifact* artifact : artifacts)
         {
+            // The main artifact must be in the list, so add it if we find it
+            if (artifact->getDesc() == desc)
+            {
+                SLANG_ASSERT(mainArtifact == nullptr);
+                mainArtifact = artifact;
+            }
+
             artifactList->add(artifact);
         }
     }
+    
+    SLANG_ASSERT(mainArtifact);
+    // Somethings gone wrong if we don't find the main artifact
+    if (!mainArtifact)
+    {
+        return SLANG_FAIL;
+    }
 
     ExecuteResult exeRes;
 
@@ -276,6 +216,39 @@ SlangResult CommandLineDownstreamCompiler::compile(const CompileOptions& inOptio
     }
 #endif
 
+    // Go through the list of artifacts in the artifactList and check if they exist. 
+    // 
+    // This is useful because `calcCompileProducts` is conservative and may produce artifacts for products that aren't actually 
+    // produced, by the compilation.
+    {
+        Count count = artifactList->getCount();
+        for (Index i = 0; i < count; ++i)
+        {
+            auto artifact = as<IArtifact>(artifactList->getAt(i));
+
+            if (!artifact->exists())
+            {
+                // We should find a file rep and if we do we can disown it. Disowning will mean
+                // when scope is lost the rep won't try and delete the (apparently non existing) backing file.
+                if (auto fileRep = findRepresentation<IFileArtifactRepresentation>(artifact))
+                {
+                    fileRep->disown();
+                }
+
+                // If the main artifact doesn't exist, we don't have a main artifact
+                if (artifact == mainArtifact)
+                {
+                    mainArtifact.setNull();
+                }
+
+                // Remove from the list
+                artifactList->removeAt(i);
+                --count;
+                --i;
+            }
+        }
+    }
+
     auto artifact = ArtifactUtil::createArtifactForCompileTarget(options.targetType);
 
     auto diagnostics = ArtifactDiagnostics::create();
@@ -284,10 +257,19 @@ SlangResult CommandLineDownstreamCompiler::compile(const CompileOptions& inOptio
 
     // Add the artifact
     artifact->addAssociated(diagnostics);
+    
+    // Find the rep from the 'main' artifact, we'll just use the same representation on the output 
+    // artifact. Sharing is needed, because the rep owns the file.
+    if (auto fileRep = mainArtifact ? findRepresentation<IFileArtifactRepresentation>(mainArtifact) : nullptr)
+    {
+        artifact->addRepresentation(fileRep);
+    }
 
-    ComPtr<IArtifactRepresentation> rep(new CommandLineDownstreamArtifactRepresentation(moduleFilePath.getUnownedSlice(), artifactList));
-    artifact->addRepresentation(rep);
-    artifact->addAssociated(artifactList);
+    // Add the artifact list if there is anything in it
+    if (artifactList->getCount())
+    {
+        artifact->addAssociated(artifactList);
+    }
 
     *outArtifact = artifact.detach();
     
diff --git a/source/compiler-core/slang-downstream-compiler.h b/source/compiler-core/slang-downstream-compiler.h
index e5e69fdfe..4fca3949a 100644
--- a/source/compiler-core/slang-downstream-compiler.h
+++ b/source/compiler-core/slang-downstream-compiler.h
@@ -227,33 +227,6 @@ public:
     Desc m_desc;
 };
 
-class CommandLineDownstreamArtifactRepresentation : public ComBaseObject, public IArtifactRepresentation
-{
-public:
-    SLANG_COM_BASE_IUNKNOWN_ALL
-
-    // ICastable
-    virtual SLANG_NO_THROW void* SLANG_MCALL castAs(const Guid& guid) SLANG_OVERRIDE;
-    // IArtifactRepresentation
-    virtual SLANG_NO_THROW SlangResult SLANG_MCALL createRepresentation(const Guid& typeGuid, ICastable** outCastable) SLANG_OVERRIDE;
-    virtual SLANG_NO_THROW bool SLANG_MCALL exists() SLANG_OVERRIDE;
-
-    CommandLineDownstreamArtifactRepresentation(const UnownedStringSlice& moduleFilePath, ICastableList* artifactList) :
-        m_moduleFilePath(moduleFilePath),
-        m_artifactList(artifactList)
-    {
-    }
-
-    ComPtr<ICastableList> m_artifactList;
-
-protected:
-
-    void* getInterface(const Guid& guid);
-    void* getObject(const Guid& guid);
-
-    String m_moduleFilePath;
-};
-
 class CommandLineDownstreamCompiler : public DownstreamCompilerBase
 {
 public:
diff --git a/source/core/slang-shared-library.h b/source/core/slang-shared-library.h
index 9015219b1..ab91a6501 100644
--- a/source/core/slang-shared-library.h
+++ b/source/core/slang-shared-library.h
@@ -81,6 +81,11 @@ class ScopeSharedLibrary : public DefaultSharedLibrary
 public: 
     typedef DefaultSharedLibrary Super;
 
+    static ComPtr<ISlangSharedLibrary> create(const SharedLibrary::Handle sharedLibraryHandle, ISlangUnknown* scope) 
+    {
+        return ComPtr< ISlangSharedLibrary>(new ScopeSharedLibrary(sharedLibraryHandle, scope)); 
+    }
+
     /// Ctor
     ScopeSharedLibrary(const SharedLibrary::Handle sharedLibraryHandle, ISlangUnknown* scope) :
         Super(sharedLibraryHandle),