Diagnostic comparison using parsing (#1683)

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

* WIP diagnostics for line number output.

* Small param naming change

* Use x macro for pass through compile human name lookup/getting.

* WIP on parsing downstream compiler output.

* Split out parsing into ParseDiagnosticUtil.
Added test result of single line.

* Dump out the std output on fail to parse diagnostics.

* Change test type for syntax-error-intrinsic.slang be TEST not TEST_DIAGNOSTIC

* Use Index for StringUtil.

* WIP: First pass support for parsing Slang diagnostics.

* WIP Testing comparing with ParseDiagnosticUtil with previous ad-hoc mechanism.

* Use the new parsing mechanism for diagnostic comparisons.

* Improvements to diagnostics parsing.
Better error handling, and fallback handling.
Added ability to parse downstream compilers without a prefix.
Added ability to parse Slang with a prefix.

6 files changed, 510 insertions, 233 deletions

diff --git a/source/core/slang-downstream-compiler.h b/source/core/slang-downstream-compiler.h
index f43c4b560..7c9831dac 100644
--- a/source/core/slang-downstream-compiler.h
+++ b/source/core/slang-downstream-compiler.h
@@ -18,6 +18,8 @@ namespace Slang
 
 struct DownstreamDiagnostic
 {
+    typedef DownstreamDiagnostic ThisType;
+
     enum class Type
     {
         Unknown,
@@ -38,6 +40,18 @@ struct DownstreamDiagnostic
         stage = Stage::Compile;
         fileLine = 0;
     }
+
+    bool operator==(const ThisType& rhs) const
+    {
+        return type == rhs.type &&
+            stage == rhs.stage &&
+            text == rhs.text &&
+            code == rhs.code &&
+            filePath == rhs.filePath &&
+            fileLine == rhs.fileLine;
+    }
+    bool operator!=(const ThisType& rhs) const { return !(*this == rhs); }
+
     static UnownedStringSlice getTypeText(Type type);
 
     Type type;                      ///< The type of error
diff --git a/source/core/slang-string-util.cpp b/source/core/slang-string-util.cpp
index a859c6945..cac7c7cfa 100644
--- a/source/core/slang-string-util.cpp
+++ b/source/core/slang-string-util.cpp
@@ -136,12 +136,12 @@ namespace Slang {
     }
 }
 
-/* static */int StringUtil::indexOfInSplit(const UnownedStringSlice& in, char splitChar, const UnownedStringSlice& find)
+/* static */Index StringUtil::indexOfInSplit(const UnownedStringSlice& in, char splitChar, const UnownedStringSlice& find)
 {
     const char* start = in.begin();
     const char* end = in.end();
 
-    for (int i = 0; start < end; ++i)
+    for (Index i = 0; start < end; ++i)
     {
         // Move cur so it's either at the end or at next split character
         const char* cur = start;
@@ -162,12 +162,12 @@ namespace Slang {
     return -1;
 }
 
-UnownedStringSlice StringUtil::getAtInSplit(const UnownedStringSlice& in, char splitChar, int index)
+UnownedStringSlice StringUtil::getAtInSplit(const UnownedStringSlice& in, char splitChar, Index index)
 {
     const char* start = in.begin();
     const char* end = in.end();
 
-    for (int i = 0; start < end; ++i)
+    for (Index i = 0; start < end; ++i)
     {
         // Move cur so it's either at the end or at next split character
         const char* cur = start;
diff --git a/source/core/slang-string-util.h b/source/core/slang-string-util.h
index dee4c7d66..dade8a61d 100644
--- a/source/core/slang-string-util.h
+++ b/source/core/slang-string-util.h
@@ -39,11 +39,11 @@ struct StringUtil
 
         /// Equivalent to doing a split and then finding the index of 'find' on the array
         /// Returns -1 if not found
-    static int indexOfInSplit(const UnownedStringSlice& in, char splitChar, const UnownedStringSlice& find);
+    static Index indexOfInSplit(const UnownedStringSlice& in, char splitChar, const UnownedStringSlice& find);
 
         /// Given the entry at the split index specified.
         /// Will return slice with begin() == nullptr if not found or input has begin() == nullptr)
-    static UnownedStringSlice getAtInSplit(const UnownedStringSlice& in, char splitChar, int index);
+    static UnownedStringSlice getAtInSplit(const UnownedStringSlice& in, char splitChar, Index index);
 
         /// Returns the size in bytes needed to hold the formatted string using the specified args, NOT including a terminating 0
         /// NOTE! The caller *should* assume this will consume the va_list (use va_copy to make a copy to be consumed)
diff --git a/tools/slang-test/parse-diagnostic-util.cpp b/tools/slang-test/parse-diagnostic-util.cpp
index af2ef2a05..6d5a64cce 100644
--- a/tools/slang-test/parse-diagnostic-util.cpp
+++ b/tools/slang-test/parse-diagnostic-util.cpp
@@ -26,22 +26,21 @@ using namespace Slang;
         const Index lineEndIndex = tail.indexOf(')');
 
         if (lineEndIndex >= 0)
-        {
+        {            
             // Extract the location info
             UnownedStringSlice locationSlice(tail.begin(), tail.begin() + lineEndIndex);
-            List<UnownedStringSlice> locationSlices;
-            StringUtil::split(locationSlice, ',', locationSlices);
 
-            Int locationLine = 0;
-            Int locationCol = 0;
+            UnownedStringSlice slices[2];
+            const Index numSlices = StringUtil::split(locationSlice, ',', 2, slices);
+            Int locationIndex[2] = { 0, 0 };
 
-            SLANG_RETURN_ON_FAIL(StringUtil::parseInt(locationSlices[0], locationLine));
-            if (locationSlices.getCount() > 1)
+            for (Index i = 0; i < numSlices; ++i)
             {
-                SLANG_RETURN_ON_FAIL(StringUtil::parseInt(locationSlices[1], locationCol));
+                SLANG_RETURN_ON_FAIL(StringUtil::parseInt(slices[i], locationIndex[i]));
             }
 
-            outDiagnostic.fileLine = locationLine;
+            // Store the line
+            outDiagnostic.fileLine = locationIndex[0];
         }
     }
     else
@@ -53,139 +52,235 @@ using namespace Slang;
 
 /* static */SlangResult ParseDiagnosticUtil::parseFXCLine(const UnownedStringSlice& line,  List<UnownedStringSlice>& lineSlices, DownstreamDiagnostic& outDiagnostic)
 {
-    SLANG_RETURN_ON_FAIL(splitPathLocation(lineSlices[1], outDiagnostic));
-
-    if (lineSlices.getCount() > 2)
+    /* tests/diagnostics/syntax-error-intrinsic.slang(14,2): error X3000: syntax error: unexpected token '@' */
+    if (lineSlices.getCount() < 3)
     {
-        UnownedStringSlice errorSlice = lineSlices[2].trim();
-        Index spaceIndex = errorSlice.indexOf(' ');
-        if (spaceIndex >= 0)
-        {
-            UnownedStringSlice diagnosticType = errorSlice.head(spaceIndex);
-            UnownedStringSlice code = errorSlice.tail(spaceIndex + 1).trim();
+        return SLANG_FAIL;
+    }
 
-            if (diagnosticType == "warning")
-            {
-                outDiagnostic.type = DownstreamDiagnostic::Type::Warning;
-            }
+    SLANG_RETURN_ON_FAIL(splitPathLocation(lineSlices[0], outDiagnostic));
 
-            outDiagnostic.code = code;
-        }
-        else
-        {
-            outDiagnostic.code = errorSlice;
-        }
+    {
+        const UnownedStringSlice errorSlice = lineSlices[1].trim();
+
+        const UnownedStringSlice errorTypeName = StringUtil::getAtInSplit(errorSlice, ' ', 0);
+        outDiagnostic.code = StringUtil::getAtInSplit(errorSlice, ' ', 1);
 
-        if (lineSlices.getCount() > 3)
+        outDiagnostic.type = DownstreamDiagnostic::Type::Error;
+        if (errorTypeName == "warning")
         {
-            outDiagnostic.text = UnownedStringSlice(lineSlices[3].begin(), line.end());
+            outDiagnostic.type = DownstreamDiagnostic::Type::Warning;
         }
     }
 
+    outDiagnostic.text = UnownedStringSlice(lineSlices[2].begin(), line.end());
     return SLANG_OK;
 }
 
 /* static */SlangResult ParseDiagnosticUtil::parseDXCLine(const UnownedStringSlice& line,  List<UnownedStringSlice>& lineSlices, DownstreamDiagnostic& outDiagnostic)
 {
-    /*     dxc: tests/diagnostics/syntax-error-intrinsic.slang:14:2: error: expected expression */
+    /* tests/diagnostics/syntax-error-intrinsic.slang:14:2: error: expected expression */
+    if (lineSlices.getCount() < 5)
+    {
+        return SLANG_FAIL;
+    }
 
-    outDiagnostic.filePath = lineSlices[1];
+    outDiagnostic.filePath = lineSlices[0];
 
-    SLANG_RETURN_ON_FAIL(StringUtil::parseInt(lineSlices[2], outDiagnostic.fileLine));
+    SLANG_RETURN_ON_FAIL(StringUtil::parseInt(lineSlices[1], outDiagnostic.fileLine));
 
     Int lineCol;
-    SLANG_RETURN_ON_FAIL(StringUtil::parseInt(lineSlices[3], lineCol));
+    SLANG_RETURN_ON_FAIL(StringUtil::parseInt(lineSlices[2], lineCol));
 
-    UnownedStringSlice typeSlice = lineSlices[4].trim();
+    UnownedStringSlice typeSlice = lineSlices[3].trim();
 
+    outDiagnostic.type = DownstreamDiagnostic::Type::Error;
     if (typeSlice == UnownedStringSlice::fromLiteral("warning"))
     {
         outDiagnostic.type = DownstreamDiagnostic::Type::Warning;
     }
 
     // The rest of the line
-    outDiagnostic.text = UnownedStringSlice(lineSlices[5].begin(), line.end());
+    outDiagnostic.text = UnownedStringSlice(lineSlices[4].begin(), line.end());
     return SLANG_OK;
 }
 
 /* static */ SlangResult ParseDiagnosticUtil::parseGlslangLine(const UnownedStringSlice& line, List<UnownedStringSlice>& lineSlices, DownstreamDiagnostic& outDiagnostic)
 {
-    if (lineSlices.getCount() < 5)
+    /* ERROR: tests/diagnostics/syntax-error-intrinsic.slang:13: '@' : unexpected token */
+
+    if (lineSlices.getCount() < 4)
     {
         return SLANG_FAIL;
     }
+    {
+        const UnownedStringSlice typeSlice = lineSlices[0].trim();
 
-    /*  glslang: ERROR: tests/diagnostics/syntax-error-intrinsic.slang:13: '@' : unexpected token
-    */
+        outDiagnostic.type = DownstreamDiagnostic::Type::Error;
+        if (typeSlice.caseInsensitiveEquals(UnownedStringSlice::fromLiteral("warning")))
+        {
+            outDiagnostic.type = DownstreamDiagnostic::Type::Warning;
+        }
+    }
+
+    outDiagnostic.filePath = lineSlices[1];
 
-    UnownedStringSlice typeSlice = lineSlices[1].trim();
-    if (typeSlice.caseInsensitiveEquals(UnownedStringSlice::fromLiteral("warning")))
+    SLANG_RETURN_ON_FAIL(StringUtil::parseInt(lineSlices[2], outDiagnostic.fileLine));
+    outDiagnostic.text = UnownedStringSlice(lineSlices[3].begin(), line.end());
+    return SLANG_OK;
+}
+
+/* static */SlangResult ParseDiagnosticUtil::parseGenericLine(const UnownedStringSlice& line, List<UnownedStringSlice>& lineSlices, DownstreamDiagnostic& outDiagnostic)
+{
+    /* e:\git\somewhere\tests\diagnostics\syntax-error-intrinsic.slang(13): error C2018:  unknown character '0x40' */
+    if (lineSlices.getCount() < 3)
     {
-        outDiagnostic.type = DownstreamDiagnostic::Type::Warning;
+        return SLANG_FAIL;
+    }
+
+    {
+        const UnownedStringSlice errorSlice = lineSlices[1].trim();
+        // Get the code
+        outDiagnostic.code = StringUtil::getAtInSplit(errorSlice, ' ', 1).trim();
+
+        const UnownedStringSlice typeSlice = StringUtil::getAtInSplit(errorSlice, ' ', 0);
+
+        outDiagnostic.type = DownstreamDiagnostic::Type::Error;
+        if (typeSlice == UnownedStringSlice::fromLiteral("warning"))
+        {
+            outDiagnostic.type = DownstreamDiagnostic::Type::Warning;
+        }
+        else if (typeSlice == UnownedStringSlice::fromLiteral("info"))
+        {
+            outDiagnostic.type = DownstreamDiagnostic::Type::Info;
+        }
     }
 
-    outDiagnostic.filePath = lineSlices[2];
+    // Get the location info
+    SLANG_RETURN_ON_FAIL(splitPathLocation(lineSlices[0], outDiagnostic));
 
-    SLANG_RETURN_ON_FAIL(StringUtil::parseInt(lineSlices[3], outDiagnostic.fileLine));
-    outDiagnostic.text = UnownedStringSlice(lineSlices[4].begin(), line.end());
+    outDiagnostic.text = UnownedStringSlice(lineSlices[2].begin(), line.end());
     return SLANG_OK;
 }
 
-/* static */SlangResult ParseDiagnosticUtil::parseGenericLine(const UnownedStringSlice& line, List<UnownedStringSlice>& lineSlices, DownstreamDiagnostic& outDiagnostic)
+static SlangResult _getSlangDiagnosticType(const UnownedStringSlice& inText, DownstreamDiagnostic::Type& outType, Int& outCode)
 {
-    /* Visual Studio 14.0: e:\git\somewhere\tests\diagnostics\syntax-error-intrinsic.slang(13): error C2018:  unknown character '0x40' */
+    UnownedStringSlice text(inText.trim());
 
-    UnownedStringSlice errorSlice = lineSlices[2].trim();
-    UnownedStringSlice typeSlice = StringUtil::getAtInSplit(errorSlice, ' ', 0);
-    if (typeSlice == UnownedStringSlice::fromLiteral("warning"))
+    static const UnownedStringSlice prefixes[] =
     {
-        outDiagnostic.type = DownstreamDiagnostic::Type::Warning;
+        UnownedStringSlice::fromLiteral("note"),
+        UnownedStringSlice::fromLiteral("warning"),
+        UnownedStringSlice::fromLiteral("error"),
+        UnownedStringSlice::fromLiteral("fatal error"),
+        UnownedStringSlice::fromLiteral("internal error"),
+        UnownedStringSlice::fromLiteral("unknown error")
+    };
+
+    Int index = -1;
+
+    for (Index i = 0; i < SLANG_COUNT_OF(prefixes); ++i)
+    {
+        const auto& prefix = prefixes[i];
+        if (text.startsWith(prefix))
+        {
+            index = i;
+            break;
+        }
     }
-    else if (typeSlice == UnownedStringSlice::fromLiteral("info"))
+
+    switch (index)
     {
-        outDiagnostic.type = DownstreamDiagnostic::Type::Info;
+        case -1:    return SLANG_FAIL;
+        case 0:     outType = DownstreamDiagnostic::Type::Info; break;
+        case 1:     outType = DownstreamDiagnostic::Type::Warning; break;
+        default:    outType = DownstreamDiagnostic::Type::Error; break;
     }
 
-    // Get the code
-    outDiagnostic.code = StringUtil::getAtInSplit(errorSlice, ' ', 1);
+    outCode = 0;
 
-    // Get the location info
-    SLANG_RETURN_ON_FAIL(splitPathLocation(lineSlices[1], outDiagnostic));
+    UnownedStringSlice tail = text.tail(prefixes[index].getLength()).trim();
+    if (tail.getLength() > 0)
+    {
+        SLANG_RETURN_ON_FAIL(StringUtil::parseInt(tail, outCode));
+    }
 
-    outDiagnostic.text = UnownedStringSlice(lineSlices[3].begin(), line.end());
+    return SLANG_OK;
+}
+
+static bool _isSlangDiagnostic(const UnownedStringSlice& line)
+{
+    /*
+    tests/diagnostics/accessors.slang(11): error 31101: accessors other than 'set' must not have parameters
+    */
+
+    UnownedStringSlice initial = StringUtil::getAtInSplit(line, ':', 0);
+
+    // Handle if path has : 
+    const Index typeIndex = (initial.getLength() == 1 && CharUtil::isAlpha(initial[0])) ? 2 : 1;
+    // Extract the type/code slice
+    UnownedStringSlice typeSlice = StringUtil::getAtInSplit(line, ':', typeIndex);
+
+    DownstreamDiagnostic::Type type;
+    Int code;
+    return SLANG_SUCCEEDED(_getSlangDiagnosticType(typeSlice, type, code));
+}
+
+/* static */SlangResult ParseDiagnosticUtil::parseSlangLine(const UnownedStringSlice& line, List<UnownedStringSlice>& lineSlices, DownstreamDiagnostic& outDiagnostic)
+{
+    /*
+    tests/diagnostics/accessors.slang(11): error 31101: accessors other than 'set' must not have parameters
+    */
+
+    // Can be larger than 3, because might be : in the actual error text
+    if (lineSlices.getCount() < 3)
+    {
+        return SLANG_FAIL;
+    }
 
+    SLANG_RETURN_ON_FAIL(splitPathLocation(lineSlices[0], outDiagnostic));
+    Int code;
+    SLANG_RETURN_ON_FAIL(_getSlangDiagnosticType(lineSlices[1], outDiagnostic.type, code));
+
+    if (code != 0)
+    {
+        StringBuilder buf;
+        buf << code;
+        outDiagnostic.code = buf.ProduceString();
+    }
+
+    outDiagnostic.text = UnownedStringSlice(lineSlices[2].begin(), line.end());
     return SLANG_OK;
 }
 
-/* static */ SlangResult ParseDiagnosticUtil::splitCompilerDiagnosticLine(SlangPassThrough downstreamCompiler, const UnownedStringSlice& line, UnownedStringSlice& linePrefix, List<UnownedStringSlice>& outSlices)
+/* static */ SlangResult ParseDiagnosticUtil::splitDiagnosticLine(const CompilerIdentity& compilerIdentity, const UnownedStringSlice& line, const UnownedStringSlice& linePrefix, List<UnownedStringSlice>& outSlices)
 {
+    StringUtil::split(line, ':', outSlices);
+
+    // If we have a prefix (typically identifying the compiler), remove so same code can be used for output with prefixes and without
+    if (linePrefix.getLength())
+    {
+        SLANG_ASSERT(outSlices[0].startsWith(linePrefix));
+        outSlices.removeAt(0);
+    }
+
     /*
     glslang: ERROR: tests/diagnostics/syntax-error-intrinsic.slang:13: '@' : unexpected token
     dxc: tests/diagnostics/syntax-error-intrinsic.slang:14:2: error: expected expression
     fxc: tests/diagnostics/syntax-error-intrinsic.slang(14,2): error X3000: syntax error: unexpected token '@'
     Visual Studio 14.0: e:\git\somewhere\tests\diagnostics\syntax-error-intrinsic.slang(13): error C2018:  unknown character '0x40'
     NVRTC 11.0: tests/diagnostics/syntax-error-intrinsic.slang(13): error : unrecognized token
+    tests/diagnostics/accessors.slang(11): error 31101: accessors other than 'set' must not have parameters
     */
 
-    Int pathIndex = 1;
-
-    StringUtil::split(line, ':', outSlices);
-
-    if (downstreamCompiler == SLANG_PASS_THROUGH_GLSLANG)
-    {
-        // If we don't have the prefix then add it
-        if (!outSlices[0].trim().startsWith(linePrefix))
-        {
-            outSlices.insert(0, linePrefix);
-        }
-        pathIndex = 2;
-    }
+    // Need to determine where the path is located, and that depends on the compiler
+    const Int pathIndex = (compilerIdentity == CompilerIdentity::make(SLANG_PASS_THROUGH_GLSLANG)) ? 1 : 0;
 
-    // Make sure this seems plausible
+    // Now we want to fix up a path as might have drive letter, and therefore :
+    // If this is the situation then we need to have a slice after the one at the index
     if (outSlices.getCount() > pathIndex + 1)
     {
-        UnownedStringSlice pathStart = outSlices[pathIndex].trim();
-
+        const UnownedStringSlice pathStart = outSlices[pathIndex].trim();
         if (pathStart.getLength() == 1 && CharUtil::isAlpha(pathStart[0]))
         {
             // Splice back together
@@ -205,6 +300,13 @@ static void _addDiagnosticNote(const UnownedStringSlice& in, List<DownstreamDiag
         return;
     }
 
+    // If there's nothing previous, we'll ignore too, as note should be in addition to
+    // a pre-existing error/warning
+    if (outDiagnostics.getCount() == 0)
+    {
+        return;
+    }
+
     // Make it a note on the output
     DownstreamDiagnostic diagnostic;
     diagnostic.reset();
@@ -229,50 +331,113 @@ static SlangResult _findDownstreamCompiler(const UnownedStringSlice& slice, Slan
     return SLANG_FAIL;
 }
 
+/* static */SlangResult ParseDiagnosticUtil::identifyCompiler(const UnownedStringSlice& inText, CompilerIdentity& outIdentity)
+{
+    outIdentity = CompilerIdentity();
+
+    // This might be overkill - we should be able to identify the compiler from the first line, of the diagnostics.
+    // Here, we go through each line trying to identify the compiler.
+    // For downstream compilers, the only way to identify unambiguously is via the compiler name prefix.
+    // For Slang we *assume* if there isn't such a prefix, and it 'looks like' a Slang diagnostic that it is
+
+    UnownedStringSlice text(inText), line;
+    while (StringUtil::extractLine(text, line))
+    {
+        UnownedStringSlice initial = StringUtil::getAtInSplit(line, ':', 0);
+
+        if (_isSlangDiagnostic(line))
+        {
+            outIdentity = CompilerIdentity::makeSlang();
+            return SLANG_OK;
+        }
+        else
+        {
+            SlangPassThrough downstreamCompiler;
+            // First entry that begins with a numeral indicates the version number
+            if (SLANG_SUCCEEDED(_findDownstreamCompiler(initial, downstreamCompiler)))
+            {
+                outIdentity = CompilerIdentity::make(downstreamCompiler);
+                return SLANG_OK;
+            }
+        }
+    }
+
+    return SLANG_FAIL;
+}
+
+/* static */ParseDiagnosticUtil::LineParser ParseDiagnosticUtil::getLineParser(const CompilerIdentity& compilerIdentity)
+{
+    if (compilerIdentity.m_type == CompilerIdentity::Slang)
+    {
+        return &parseSlangLine;
+    }
+    else if (compilerIdentity.m_type == CompilerIdentity::DownstreamCompiler)
+    {
+        switch (compilerIdentity.m_downstreamCompiler)
+        {
+            case SLANG_PASS_THROUGH_FXC:        return &parseFXCLine;
+            case SLANG_PASS_THROUGH_DXC:        return &parseDXCLine;
+            case SLANG_PASS_THROUGH_GLSLANG:    return &parseGlslangLine;
+            default:                            return &parseGenericLine;
+        }
+    }
+    return nullptr;
+}
+
 /* static */SlangResult ParseDiagnosticUtil::parseDiagnostics(const UnownedStringSlice& inText, List<DownstreamDiagnostic>& outDiagnostics)
 {
     // TODO(JS):
-    // As it stands output of downstream compilers isn't standardized. This should be improved upon, and perhaps
+    // As it stands output of downstream compilers isn't standardized. This can be improved upon - and if so
     // we should have a function that will parse the standardized output
     // Currently dxc/fxc/glslang, use a different downstream path
 
-    // We look for the first l
+    CompilerIdentity compilerIdentity;
+    SLANG_RETURN_ON_FAIL(ParseDiagnosticUtil::identifyCompiler(inText, compilerIdentity));
 
-    SlangPassThrough downstreamCompiler = SLANG_PASS_THROUGH_NONE;
     UnownedStringSlice linePrefix;
 
+    if (compilerIdentity.m_type == CompilerIdentity::Type::DownstreamCompiler)
+    {
+        linePrefix = TypeTextUtil::getPassThroughAsHumanText(compilerIdentity.m_downstreamCompiler);
+    }
+    else
+    {
+        // For Slang there isn't *currently* a prefix ever used, but that might change in the future
+        // For now we assume no prefix.
+    }
+
+    return parseDiagnostics(inText, compilerIdentity, linePrefix, outDiagnostics);
+}
+
+/* static */SlangResult ParseDiagnosticUtil::parseDiagnostics(const UnownedStringSlice& inText, const CompilerIdentity& compilerIdentity, const UnownedStringSlice& linePrefix, List<DownstreamDiagnostic>& outDiagnostics)
+{
+    auto lineParser = getLineParser(compilerIdentity);
+    if (!lineParser)
+    {
+        return SLANG_FAIL;
+    }
+
     List<UnownedStringSlice> splitLine;
 
     UnownedStringSlice text(inText), line;
     while (StringUtil::extractLine(text, line))
     {
-        UnownedStringSlice initial = StringUtil::getAtInSplit(line, ':', 0);
-       
-        if (downstreamCompiler == SLANG_PASS_THROUGH_NONE)
+        bool isValidSplit = false;
+        // And the first entry must contain the prefix, else assume it's a note
+        if (linePrefix.getLength() > 0 && line.startsWith(linePrefix))
         {
-            // First entry that begins with a numeral indicates the version number
-            if (SLANG_FAILED(_findDownstreamCompiler(initial, downstreamCompiler)))
-            {
-                continue;
-            }
-         
-            linePrefix = TypeTextUtil::getPassThroughAsHumanText(downstreamCompiler);
+            // Try with the line prefix
+            isValidSplit = SLANG_SUCCEEDED(splitDiagnosticLine(compilerIdentity, line, linePrefix, splitLine));
         }
 
-        if (line.indexOf(':') < 0 )
+        if (!isValidSplit)
         {
-            _addDiagnosticNote(line, outDiagnostics);
-            continue;
-        }
-
-        if (SLANG_FAILED(splitCompilerDiagnosticLine(downstreamCompiler, line, linePrefix, splitLine)))
-        {
-            _addDiagnosticNote(line, outDiagnostics);
-            continue;
+            // Try without the prefix, as some output output's only some lines with the prefix (GLSL for example)
+            isValidSplit = SLANG_SUCCEEDED(splitDiagnosticLine(compilerIdentity, line, UnownedStringSlice(), splitLine));
         }
 
-        // If doesn't have prefix, just add as note
-        if (!splitLine[0].trim().startsWith(linePrefix))
+        // If we don't have a valid split then just assume it's a note
+        if (!isValidSplit)
         {
             _addDiagnosticNote(line, outDiagnostics);
             continue;
@@ -283,42 +448,160 @@ static SlangResult _findDownstreamCompiler(const UnownedStringSlice& slice, Slan
         diagnostic.stage = DownstreamDiagnostic::Stage::Compile;
         diagnostic.fileLine = 0;
         
-        SlangResult parseRes;
+        if (SLANG_SUCCEEDED(lineParser(line, splitLine, diagnostic)))
+        {
+            outDiagnostics.add(diagnostic);   
+        }
+        else
+        {
+            // If couldn't parse, just add as a note
+            _addDiagnosticNote(line, outDiagnostics);
+        }
+    }
+
+    return SLANG_OK;
+}
+
+static UnownedStringSlice _getEquals(const UnownedStringSlice& in)
+{
+    Index equalsIndex = in.indexOf('=');
+    if (equalsIndex < 0)
+    {
+        return UnownedStringSlice();
+    }
+    return in.tail(equalsIndex + 1).trim();
+}
+
+/* static */SlangResult ParseDiagnosticUtil::parseOutputInfo(const UnownedStringSlice& inText, OutputInfo& out)
+{
+    enum State
+    {
+        Normal,
+        InStdError,
+        InStdOut,
+    };
+
+    UnownedStringSlice resultCodePrefix = UnownedStringSlice::fromLiteral("result code");
+    UnownedStringSlice stdErrorPrefix = UnownedStringSlice::fromLiteral("standard error");
+    UnownedStringSlice stdOutputPrefix = UnownedStringSlice::fromLiteral("standard output");
+
+    
+    List<UnownedStringSlice> lines;
 
-        switch (downstreamCompiler)
+    State state = State::Normal;
+
+    UnownedStringSlice text(inText), line;
+    while (StringUtil::extractLine(text, line))
+    {
+        switch (state)
         {
-            case SLANG_PASS_THROUGH_FXC:
+            case State::Normal:
             {
-                parseRes = parseFXCLine(line, splitLine, diagnostic);
+                if (line.startsWith(resultCodePrefix))
+                {
+                    // Split past the equal
+                    const UnownedStringSlice valueSlice = _getEquals(line.tail(resultCodePrefix.getLength()));
+                    Int value;
+                    SLANG_RETURN_ON_FAIL(StringUtil::parseInt(valueSlice, value));
+                    out.resultCode = int(value);
+                }
+                else
+                {
+                    UnownedStringSlice* startsWith = nullptr;
+                    if (line.startsWith(stdErrorPrefix))
+                    {
+                        startsWith = &stdErrorPrefix;
+                    }
+                    else if (line.startsWith(stdOutputPrefix))
+                    {
+                        startsWith = &stdOutputPrefix;
+                    }
+
+                    if (startsWith)
+                    {
+                        // Clear the lines buffer
+                        lines.clear();
+
+                        UnownedStringSlice valueSlice = _getEquals(line.tail(startsWith->getLength()));
+                        if (!valueSlice.isChar('{'))
+                        {
+                            return SLANG_FAIL;
+                        }
+                        // Okay we now inside std out or std error, so update the state
+                        state = (startsWith == &stdErrorPrefix) ? State::InStdError : State::InStdOut;
+                    }
+                }
                 break;
             }
-            case SLANG_PASS_THROUGH_DXC:
+            case State::InStdError:
+            case State::InStdOut:
             {
-                parseRes = parseDXCLine(line, splitLine, diagnostic);
-                break;
+                if (line == "}")
+                {
+                    String& dst = state == State::InStdError ? out.stdError : out.stdOut;
+                    if (lines.getCount() > 0)
+                    {
+                        dst = UnownedStringSlice(lines[0].begin(), lines.getLast().end());
+                    }
+                    state = State::Normal;
+                }
+                else
+                {
+                    lines.add(line);
+                }
             }
-            case SLANG_PASS_THROUGH_GLSLANG:
+        }
+    }
+
+    return (state == State::Normal) ? SLANG_OK : SLANG_FAIL;
+}
+
+
+/* static */bool ParseDiagnosticUtil::areEqual(const UnownedStringSlice& a, const UnownedStringSlice& b, EqualityFlags flags)
+{
+    List<DownstreamDiagnostic> diagsA, diagsB;
+    SlangResult resA = ParseDiagnosticUtil::parseDiagnostics(a, diagsA);
+    SlangResult resB = ParseDiagnosticUtil::parseDiagnostics(b, diagsB);
+
+    /*
+        TODO(JS): In the past we needed special handling of the stdlib, when
+        in some builds the path contains the stdlib.
+
+        For now we don't seem to need this, this is for future reference, if there
+        is an issue with needing to specially handle this.
+
+       static const UnownedStringSlice stdLibNames[] =
+        {
+            UnownedStringSlice::fromLiteral("core.meta.slang"),
+            UnownedStringSlice::fromLiteral("hlsl.meta.slang"),
+            UnownedStringSlice::fromLiteral("slang-stdlib.cpp"),
+        };
+        */
+
+    // Must have both succeeded, and have the same amount of lines
+    if (SLANG_SUCCEEDED(resA) && SLANG_SUCCEEDED(resB) &&
+        diagsA.getCount() == diagsB.getCount())
+    {
+        for (Index i = 0; i < diagsA.getCount(); ++i)
+        {
+            DownstreamDiagnostic diagA = diagsA[i];
+            DownstreamDiagnostic diagB = diagsB[i];
+
+            // Check if we need to ignore line numbers
+            if (flags & EqualityFlag::IgnoreLineNos)
             {
-                parseRes = parseGlslangLine(line, splitLine, diagnostic);
-                break;
+                diagA.fileLine = 0;
+                diagB.fileLine = 0;
             }
-            default:
+
+            if (diagA != diagB)
             {
-                parseRes = parseGenericLine(line, splitLine, diagnostic);
-                break;
+                return false;
             }
         }
 
-        // If couldn't parse, just add as a note
-        if (SLANG_FAILED(parseRes))
-        {
-            _addDiagnosticNote(line, outDiagnostics);
-        }
-        else
-        {
-            outDiagnostics.add(diagnostic);
-        }
+        return true;
     }
-
-    return SLANG_OK;
+    
+    return false;
 }
diff --git a/tools/slang-test/parse-diagnostic-util.h b/tools/slang-test/parse-diagnostic-util.h
index 26349b779..13d1bdf70 100644
--- a/tools/slang-test/parse-diagnostic-util.h
+++ b/tools/slang-test/parse-diagnostic-util.h
@@ -11,20 +11,91 @@
 
 struct ParseDiagnosticUtil
 {
+    struct OutputInfo
+    {
+        int resultCode;
+        Slang::String stdError;
+        Slang::String stdOut;
+    };
+
+        /// We need a way to identify downstream compilers and others - specifically here Slang.
+        /// Ideally we'd have an enum that included Slang.
+        /// Just adding to SlangPassThrough doesn't seem right. If we had an enumeration with a
+        /// more appropriate name, then including downstream and slang compilers wouldn't be a problem.
+        /// So for now this is punted, and this type is used to represent possible compiler identities. 
+    struct CompilerIdentity
+    {
+        typedef CompilerIdentity ThisType;
+
+        enum Type
+        {
+            Unknown,
+            Slang,
+            DownstreamCompiler,
+        };
+
+        static CompilerIdentity make(Type type, SlangPassThrough downstreamCompiler) { CompilerIdentity ident; ident.m_type = type; ident.m_downstreamCompiler = downstreamCompiler; return ident; }
+        static CompilerIdentity make(SlangPassThrough downstreamCompiler) { return make(Type::DownstreamCompiler, downstreamCompiler); }
+        static CompilerIdentity makeSlang() { return make(Type::Slang, SLANG_PASS_THROUGH_NONE); }
+
+        bool operator==(const ThisType& rhs) const { return m_type == rhs.m_type && m_downstreamCompiler == rhs.m_downstreamCompiler; }
+        bool operator!=(const ThisType& rhs) const { return !(*this == rhs); }
+
+        Type m_type = Type::Unknown; 
+        SlangPassThrough m_downstreamCompiler = SLANG_PASS_THROUGH_NONE;
+    };
+
+    typedef uint32_t EqualityFlags;
+    struct EqualityFlag
+    {
+        enum Enum : EqualityFlags
+        {
+            IgnoreLineNos = 0x1,
+        };
+    };
+
+    typedef SlangResult (*LineParser)(const Slang::UnownedStringSlice& line, Slang::List<Slang::UnownedStringSlice>& lineSlices, Slang::DownstreamDiagnostic& outDiagnostic);
+
+        /// Given a compiler identity returns a line parsing function.
+    static LineParser getLineParser(const CompilerIdentity& compilerIdentity);
+
+        /// Given a path, that holds line number and potentially column number in () after path, writes result into outDiagnostic
     static SlangResult splitPathLocation(const Slang::UnownedStringSlice& pathLocation, Slang::DownstreamDiagnostic& outDiagnostic);
 
+        /// For FXC lines
     static SlangResult parseFXCLine(const Slang::UnownedStringSlice& line, Slang::List<Slang::UnownedStringSlice>& lineSlices, Slang::DownstreamDiagnostic& outDiagnostic);
 
+        /// For DXC lines
     static SlangResult parseDXCLine(const Slang::UnownedStringSlice& line, Slang::List<Slang::UnownedStringSlice>& lineSlices, Slang::DownstreamDiagnostic& outDiagnostic);
 
+        /// For GLSL lines
     static SlangResult parseGlslangLine(const Slang::UnownedStringSlice& line, Slang::List<Slang::UnownedStringSlice>& lineSlices, Slang::DownstreamDiagnostic& outDiagnostic);
 
+        /// For a 'generic' (as in uses DownstreamCompiler mechanism) line parsing
     static SlangResult parseGenericLine(const Slang::UnownedStringSlice& line, Slang::List<Slang::UnownedStringSlice>& lineSlices, Slang::DownstreamDiagnostic& outDiagnostic);
 
-    static SlangResult splitCompilerDiagnosticLine(SlangPassThrough downstreamCompiler, const Slang::UnownedStringSlice& line, Slang::UnownedStringSlice& linePrefix, Slang::List<Slang::UnownedStringSlice>& outSlices);
+        /// For parsing diagnostics from Slang
+    static SlangResult parseSlangLine(const Slang::UnownedStringSlice& line, Slang::List<Slang::UnownedStringSlice>& lineSlices, Slang::DownstreamDiagnostic& outDiagnostic);
 
+        /// Parse diagnostics into output text
     static SlangResult parseDiagnostics(const Slang::UnownedStringSlice& inText, Slang::List<Slang::DownstreamDiagnostic>& outDiagnostics);
 
+        /// Parse diagnostics with known compiler identity.
+        /// If the prefix is empty, it is assumed there is no prefix and it won't be checked.
+    static SlangResult parseDiagnostics(const Slang::UnownedStringSlice& inText, const CompilerIdentity& identity, const Slang::UnownedStringSlice& prefix, Slang::List<Slang::DownstreamDiagnostic>& outDiagnostics);
+
+        /// Given the file output style used by tests, get components of the output into Diagnostic
+    static SlangResult parseOutputInfo(const Slang::UnownedStringSlice& in, OutputInfo& out);
+
+        /// Given a line split it into slices - taking into account compiler output, path considerations, and potentially line prefixing
+    static SlangResult splitDiagnosticLine(const CompilerIdentity& compilerIdentity, const Slang::UnownedStringSlice& line, const Slang::UnownedStringSlice& linePrefix, Slang::List<Slang::UnownedStringSlice>& outSlices);
+
+        /// Give text of diagnostic determine which compiler the output is from
+    static SlangResult identifyCompiler(const Slang::UnownedStringSlice& in, CompilerIdentity& outIdentity);
+
+        /// Determines if the diagnostics in a and b (they are parsed via parseDiagnostics) are equal, taking into account flags
+        /// Note! If the parse of either a or b fails, then equality is returns as false.
+    static bool areEqual(const Slang::UnownedStringSlice& a, const Slang::UnownedStringSlice& b, EqualityFlags flags);
 };
 
 #endif // PARSE_DIAGNOSTIC_UTIL_H
diff --git a/tools/slang-test/slang-test-main.cpp b/tools/slang-test/slang-test-main.cpp
index fcb609c34..e7e882ad4 100644
--- a/tools/slang-test/slang-test-main.cpp
+++ b/tools/slang-test/slang-test-main.cpp
@@ -1028,120 +1028,29 @@ static SlangResult _executeBinary(const UnownedStringSlice& hexDump, ExecuteResu
     return ProcessUtil::execute(cmdLine, outExeRes);
 }
 
-static UnownedStringSlice _removeDiagnosticPrefix(const UnownedStringSlice& prefix, const UnownedStringSlice& in)
-{
-    SLANG_ASSERT(in.startsWith(prefix));
-
-    UnownedStringSlice remaining(in.begin() + prefix.getLength(), in.end());
-    const Index index = remaining.indexOf(':');
-
-    if (index >= 0)
-    {
-        // Ok strip everything before the colon
-        return UnownedStringSlice(remaining.begin() + index, remaining.end());
-    }
-    else
-    {
-        // Couldn't strip, just return the whole string as is
-        return in;
-    }
-}
-
-static bool _isUnsignedInteger(const UnownedStringSlice& a)
-{
-    const char* end = a.end();
-    for (const char* cur = a.begin(); cur < end; ++cur)
-    {
-        if (!(*cur >= '0' && *cur <= '9'))
-        {
-            return false;
-        }
-    }
-    return true;
-}
-
-static bool _isDXCLineSplitEqual(const UnownedStringSlice& a, const UnownedStringSlice& b)
-{
-    return a == b || (_isUnsignedInteger(a.trim()) && _isUnsignedInteger(b.trim()));
-}
-
-// Returns true if a and b are output from dxc (prefixed with dxc:.
-// Ignores line number/column number differences from the dxc specific line format. 
-static bool _isDXCLineEqual(const UnownedStringSlice& a, const UnownedStringSlice& b)
-{
-    // We are going to ignore the line number/column number.
-    // To do this if we find any sub strings inbetween : that are just all digits we'll assume it's a line number/column number
-    // and ignore
-
-    // dxc: tests/cross-compile/dxc-error.hlsl:9:2: error: use of undeclared identifier 'gOutputBuffer'
-    const UnownedStringSlice dxcPrefix = UnownedStringSlice::fromLiteral("dxc:");
-    return a.startsWith(dxcPrefix) && b.startsWith(dxcPrefix) && StringUtil::areAllEqualWithSplit(a, b, ':', _isDXCLineSplitEqual);
-}
-
-static bool _isLineEqual(const UnownedStringSlice& a, const UnownedStringSlice& b)
-{
-    if (a == b)
-    {
-        return true;
-    }
-
-    static const UnownedStringSlice stdLibNames[] =
-    {
-        UnownedStringSlice::fromLiteral("core.meta.slang"),
-        UnownedStringSlice::fromLiteral("hlsl.meta.slang"),
-        UnownedStringSlice::fromLiteral("slang-stdlib.cpp"),
-    };
-
-    // Look for if a line starts with a stdlib name
-    for (const auto& stdLibName : stdLibNames)
-    {
-        if (a.startsWith(stdLibName) && b.startsWith(stdLibName))
-        {
-            // If the text after the diagnostic prefix is equal then the line is equal
-            if (_removeDiagnosticPrefix(stdLibName, a) == _removeDiagnosticPrefix(stdLibName, b))
-            {
-                return true;
-            }
-        }
-    }
-
-    return _isDXCLineEqual(a, b);
-}
-
 static bool _areDiagnosticsEqual(const UnownedStringSlice& a, const UnownedStringSlice& b)
 {
-    // If they are identical we are done
-    if (a == b)
-    {
-        return true;
-    }
-
-    // Okay we are going to go line by line
-    // If the lines are equal thats ok.
-    // If they are not.. we will check if the only difference is line numbers from the stdlib
-
-    List<UnownedStringSlice> linesA;
-    List<UnownedStringSlice> linesB;
-
-    StringUtil::calcLines(a, linesA);
-    StringUtil::calcLines(b, linesB);
+    ParseDiagnosticUtil::OutputInfo outA, outB;
 
-    if (linesA.getCount() != linesB.getCount())
+    // If we can't parse, we can't match, so fail.
+    if (SLANG_FAILED(ParseDiagnosticUtil::parseOutputInfo(a, outA)) ||
+        SLANG_FAILED(ParseDiagnosticUtil::parseOutputInfo(b, outB)))
     {
         return false;
     }
 
-    for (Index i = 0; i < linesA.getCount(); ++i)
+    // The result codes must match, and std out
+    if (outA.resultCode != outB.resultCode ||
+        !StringUtil::areLinesEqual(outA.stdOut.getUnownedSlice(), outB.stdOut.getUnownedSlice()))
     {
-        if (!_isLineEqual(linesA[i], linesB[i]))
-        {
-            return false;
-        }
+        return false;
     }
 
-    return true;
+    // Parse the compiler diagnostics and make sure they are the same.
+    // Ignores line number differences 
+    return ParseDiagnosticUtil::areEqual(outA.stdError.getUnownedSlice(), outB.stdError.getUnownedSlice(), ParseDiagnosticUtil::EqualityFlag::IgnoreLineNos);
 }
-
+   
 static bool _areResultsEqual(TestOptions::Type type, const String& a, const String& b)
 {
     switch (type)