Improvements to repro diagnostics (#2039)

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

* Improvements to repro diagnostics.

* Fix typo.

7 files changed, 81 insertions, 22 deletions

diff --git a/source/core/slang-riff.h b/source/core/slang-riff.h
index 83a522e81..3a964df2d 100644
--- a/source/core/slang-riff.h
+++ b/source/core/slang-riff.h
@@ -5,6 +5,7 @@
 #include "slang-stream.h"
 #include "slang-memory-arena.h"
 #include "slang-writer.h"
+#include "slang-semantic-version.h"
 
 namespace Slang
 {
@@ -88,6 +89,11 @@ struct RiffSemanticVersion
         /// A major change is binary incompatible by default
     int getMajor() const { return (m_raw >> 16); }
 
+    SemanticVersion asSemanticVersion() const
+    {
+        return SemanticVersion(getMajor(), getMinor(), getPatch());
+    }
+
     static RawType makeRaw(int major, int minor, int patch)
     {
         SLANG_ASSERT((major | minor | patch) >= 0);
@@ -103,6 +109,7 @@ struct RiffSemanticVersion
     }
 
     static RiffSemanticVersion make(int major, int minor, int patch) { return makeFromRaw(makeRaw(major, minor, patch)); }
+    static RiffSemanticVersion make(const SemanticVersion& in) { return makeFromRaw(makeRaw(in.m_major, in.m_minor, in.m_patch)); }
 
         /// True if the read version is compatible with the current version, based on semantic rules.
     static bool areCompatible(const ThisType& currentVersion, const ThisType& readVersion)
diff --git a/source/slang/slang-api.cpp b/source/slang/slang-api.cpp
index 5ee384e4a..e8e9b602d 100644
--- a/source/slang/slang-api.cpp
+++ b/source/slang/slang-api.cpp
@@ -777,10 +777,13 @@ SLANG_API SlangResult spExtractRepro(SlangSession* session, const void* reproDat
     using namespace Slang;
     SLANG_UNUSED(session);
 
+    DiagnosticSink sink;
+    sink.init(nullptr, nullptr);
+
     List<uint8_t> buffer;
     {
         MemoryStreamBase memoryStream(FileAccess::Read, reproData, reproDataSize);
-        SLANG_RETURN_ON_FAIL(ReproUtil::loadState(&memoryStream, buffer));
+        SLANG_RETURN_ON_FAIL(ReproUtil::loadState(&memoryStream, &sink, buffer));
     }
 
     MemoryOffsetBase base;
@@ -801,10 +804,13 @@ SLANG_API SlangResult spLoadReproAsFileSystem(
 
     SLANG_UNUSED(session);
 
+    DiagnosticSink sink;
+    sink.init(nullptr, nullptr);
+
     MemoryStreamBase stream(FileAccess::Read, reproData, reproDataSize);
 
     List<uint8_t> buffer;
-    SLANG_RETURN_ON_FAIL(ReproUtil::loadState(&stream, buffer));
+    SLANG_RETURN_ON_FAIL(ReproUtil::loadState(&stream, &sink, buffer));
 
     auto requestState = ReproUtil::getRequest(buffer);
     MemoryOffsetBase base;
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index 897aa9f6f..f26410633 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -118,6 +118,12 @@ DIAGNOSTIC(    86, Error, unableToCreateModuleContainer, "unable to create modul
 DIAGNOSTIC(    87, Error, unableToSetDefaultDownstreamCompiler, "unable to set default downstream compiler for source language '%0' to '%1'")
 
 DIAGNOSTIC(    88, Error, unknownArchiveType, "archive type '%0' is unknown")
+DIAGNOSTIC(    89, Error, expectingSlangRiffContainer, "expecting a slang riff container")
+DIAGNOSTIC(    90, Error, incompatibleRiffSemanticVersion, "incompatible riff semantic version %0 expecting %1")
+DIAGNOSTIC(    91, Error, riffHashMismatch, "riff hash mismatch - incompatible riff")
+DIAGNOSTIC(    92, Error, unableToCreateDirectory, "unable to create directory '$0'")
+DIAGNOSTIC(    93, Error, unableExtractReproToDirectory, "unable to extract repro to directory '$0'")
+DIAGNOSTIC(    94, Error, unableToReadRiff, "unable to read as 'riff'/not a 'riff' file")
 
 //
 // 001xx - Downstream Compilers
diff --git a/source/slang/slang-options.cpp b/source/slang/slang-options.cpp
index 1c7d8fbc2..8f5c75efa 100644
--- a/source/slang/slang-options.cpp
+++ b/source/slang/slang-options.cpp
@@ -413,7 +413,7 @@ struct OptionsParser
         Slang::List<String> m_filenames;
     };
 
-    static SlangResult _compileReproDirectory(SlangSession* session, EndToEndCompileRequest* originalRequest, const String& dir)
+    static SlangResult _compileReproDirectory(SlangSession* session, EndToEndCompileRequest* originalRequest, const String& dir, DiagnosticSink* sink)
     {
         auto stdOut = originalRequest->getWriter(WriterChannel::StdOutput);
 
@@ -430,7 +430,7 @@ struct OptionsParser
             auto requestImpl = asInternal(request);
 
             List<uint8_t> buffer;
-            SLANG_RETURN_ON_FAIL(ReproUtil::loadState(path, buffer));
+            SLANG_RETURN_ON_FAIL(ReproUtil::loadState(path, sink, buffer));
 
             auto requestState = ReproUtil::getRequest(buffer);
             MemoryOffsetBase base;
@@ -671,7 +671,14 @@ struct OptionsParser
                     CommandLineArg reproName;
                     SLANG_RETURN_ON_FAIL(reader.expectArg(reproName));
 
-                    SLANG_RETURN_ON_FAIL(ReproUtil::extractFilesToDirectory(reproName.value));
+                    {
+                        const Result res = ReproUtil::extractFilesToDirectory(reproName.value, sink);
+                        if (SLANG_FAILED(res))
+                        {
+                            sink->diagnose(reproName.loc, Diagnostics::unableExtractReproToDirectory, reproName.value);
+                            return res;
+                        }
+                    }
                 }
                 else if (argValue == "-module-name")
                 {
@@ -686,7 +693,14 @@ struct OptionsParser
                     SLANG_RETURN_ON_FAIL(reader.expectArg(reproName));
 
                     List<uint8_t> buffer;
-                    SLANG_RETURN_ON_FAIL(ReproUtil::loadState(reproName.value, buffer));
+                    {
+                        const Result res = ReproUtil::loadState(reproName.value, sink, buffer);
+                        if (SLANG_FAILED(res))
+                        {
+                            sink->diagnose(reproName.loc, Diagnostics::unableToReadFile, reproName.value);
+                            return res;
+                        }
+                    }
 
                     auto requestState = ReproUtil::getRequest(buffer);
                     MemoryOffsetBase base;
@@ -713,7 +727,7 @@ struct OptionsParser
                     CommandLineArg reproDirectory;
                     SLANG_RETURN_ON_FAIL(reader.expectArg(reproDirectory));
 
-                    SLANG_RETURN_ON_FAIL(_compileReproDirectory(session, requestImpl, reproDirectory.value));
+                    SLANG_RETURN_ON_FAIL(_compileReproDirectory(session, requestImpl, reproDirectory.value, sink));
                 }
                 else if (argValue == "-repro-file-system")
                 {
@@ -721,7 +735,14 @@ struct OptionsParser
                     SLANG_RETURN_ON_FAIL(reader.expectArg(reproName));
 
                     List<uint8_t> buffer;
-                    SLANG_RETURN_ON_FAIL(ReproUtil::loadState(reproName.value, buffer));
+                    {
+                        const Result res = ReproUtil::loadState(reproName.value, sink, buffer);
+                        if (SLANG_FAILED(res))
+                        {
+                            sink->diagnose(reproName.loc, Diagnostics::unableToReadFile, reproName.value);
+                            return res;
+                        }
+                    }
 
                     auto requestState = ReproUtil::getRequest(buffer);
                     MemoryOffsetBase base;
diff --git a/source/slang/slang-repro.cpp b/source/slang/slang-repro.cpp
index d7ddd6aa8..a5591e68b 100644
--- a/source/slang/slang-repro.cpp
+++ b/source/slang/slang-repro.cpp
@@ -1087,40 +1087,54 @@ struct LoadContext
     return saveState(request, stream);
 }
 
-/* static */ SlangResult ReproUtil::loadState(const String& filename, List<uint8_t>& outBuffer)
+/* static */ SlangResult ReproUtil::loadState(const String& filename, DiagnosticSink* sink, List<uint8_t>& outBuffer)
 {
     RefPtr<FileStream> stream = new FileStream;
     SLANG_RETURN_ON_FAIL(stream->init(filename, FileMode::Open, FileAccess::Read, FileShare::ReadWrite));
-    return loadState(stream, outBuffer);
+    return loadState(stream, sink, outBuffer);
 }
 
-/* static */ SlangResult ReproUtil::loadState(Stream* stream, List<uint8_t>& buffer)
+/* static */ SlangResult ReproUtil::loadState(Stream* stream, DiagnosticSink* sink, List<uint8_t>& buffer)
 {
     Header header;
 
-    SLANG_RETURN_ON_FAIL(RiffUtil::readData(stream, &header.m_chunk, sizeof(header), buffer));
+    {
+        Result res = RiffUtil::readData(stream, &header.m_chunk, sizeof(header), buffer);
+        if (SLANG_FAILED(res))
+        {
+            sink->diagnose(SourceLoc(), Diagnostics::unableToReadRiff);
+            return res;
+        }
+    }
     if (header.m_chunk.type != kSlangStateFourCC)
     {
+        sink->diagnose(SourceLoc(), Diagnostics::expectingSlangRiffContainer);
         return SLANG_FAIL;
     }
 
     if (!RiffSemanticVersion::areCompatible(g_semanticVersion, header.m_semanticVersion))
     {
+        StringBuilder headerBuf, currentBuf;
+        header.m_semanticVersion.asSemanticVersion().append(headerBuf);
+        g_semanticVersion.asSemanticVersion().append(currentBuf);
+
+        sink->diagnose(SourceLoc(), Diagnostics::incompatibleRiffSemanticVersion, headerBuf, currentBuf);
         return SLANG_FAIL;
     }
 
     if (header.m_typeHash != uint32_t(_getTypeHash()))
     {
+        sink->diagnose(SourceLoc(), Diagnostics::riffHashMismatch);
         return SLANG_FAIL;
     }
 
     return SLANG_OK;
 }
 
-/* static */SlangResult ReproUtil::loadState(const uint8_t* data, size_t size, List<uint8_t>& outBuffer)
+/* static */SlangResult ReproUtil::loadState(const uint8_t* data, size_t size, DiagnosticSink* sink, List<uint8_t>& outBuffer)
 {
     MemoryStreamBase stream(FileAccess::Read, data, size);
-    return loadState(&stream, outBuffer);
+    return loadState(&stream, sink, outBuffer);
 }
 
 /* static */ ReproUtil::RequestState* ReproUtil::getRequest(const List<uint8_t>& buffer)
@@ -1149,10 +1163,10 @@ struct LoadContext
     return SLANG_OK;
 }
 
-/* static */SlangResult ReproUtil::extractFilesToDirectory(const String& filename)
+/* static */SlangResult ReproUtil::extractFilesToDirectory(const String& filename, DiagnosticSink* sink)
 {
     List<uint8_t> buffer;
-    SLANG_RETURN_ON_FAIL(ReproUtil::loadState(filename, buffer));
+    SLANG_RETURN_ON_FAIL(ReproUtil::loadState(filename, sink, buffer));
 
     MemoryOffsetBase base;
     base.set(buffer.getBuffer(), buffer.getCount());
@@ -1162,7 +1176,12 @@ struct LoadContext
     String dirPath;
     SLANG_RETURN_ON_FAIL(ReproUtil::calcDirectoryPathFromFilename(filename, dirPath));
 
-    Path::createDirectory(dirPath);
+    if (!Path::createDirectory(dirPath))
+    {
+        sink->diagnose(SourceLoc(), Diagnostics::unableToCreateDirectory, dirPath);
+        return SLANG_FAIL;
+    }
+
     // Set up a file system to write into this directory
     RelativeFileSystem relFileSystem(OSFileSystem::getMutableSingleton(), dirPath);
 
diff --git a/source/slang/slang-repro.h b/source/slang/slang-repro.h
index 38a76a50a..7a192743f 100644
--- a/source/slang/slang-repro.h
+++ b/source/slang/slang-repro.h
@@ -177,13 +177,13 @@ struct ReproUtil
         /// it will attempt to load from fileSystem the *uniqueName*
     static SlangResult load(OffsetBase& base, RequestState* requestState, ISlangFileSystem* fileSystem, EndToEndCompileRequest* request);
 
-    static SlangResult loadState(const String& filename, List<uint8_t>& outBuffer);
-    static SlangResult loadState(Stream* stream, List<uint8_t>& outBuffer);
-    static SlangResult loadState(const uint8_t* data, size_t size, List<uint8_t>& outBuffer);
+    static SlangResult loadState(const String& filename, DiagnosticSink* sink, List<uint8_t>& outBuffer);
+    static SlangResult loadState(Stream* stream, DiagnosticSink* sink, List<uint8_t>& outBuffer);
+    static SlangResult loadState(const uint8_t* data, size_t size, DiagnosticSink* sink, List<uint8_t>& outBuffer);
 
     static RequestState* getRequest(const List<uint8_t>& inBuffer);
 
-    static SlangResult extractFilesToDirectory(const String& file);
+    static SlangResult extractFilesToDirectory(const String& file, DiagnosticSink* sink);
 
     static SlangResult extractFiles(OffsetBase& base, RequestState* requestState, ISlangMutableFileSystem* fileSystem);
 
diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
index 4696d50af..bef6bd141 100644
--- a/source/slang/slang.cpp
+++ b/source/slang/slang.cpp
@@ -4678,7 +4678,7 @@ SlangResult EndToEndCompileRequest::getContainerCode(ISlangBlob** outBlob)
 SlangResult EndToEndCompileRequest::loadRepro(ISlangFileSystem* fileSystem, const void* data, size_t size)
 {
     List<uint8_t> buffer;
-    SLANG_RETURN_ON_FAIL(ReproUtil::loadState((const uint8_t*)data, size, buffer));
+    SLANG_RETURN_ON_FAIL(ReproUtil::loadState((const uint8_t*)data, size, getSink(), buffer));
 
     MemoryOffsetBase base;
     base.set(buffer.getBuffer(), buffer.getCount());