Simplify CommandLine by removing Escaping (#1825)

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

* Split out StringEscapeUtil.

* Added StringEscapeUtil.

* Fix typo in unix quoting type.

* Small comment improvements.

* Try to fix linux linking issue.

* Fix typo.

* Attempt to fix linux link issue.

* Update VS proj even though nothing really changed.

* Fix another typo issue.

* Fix for windows issue.
Fixed bug.

* Make separate Utils for escaping.

* Fix typo.

* Split out into StringEscapeHandler.

* Windows shell does handle removing quotes (so remove code to remove them).

* Handle unescaping if not initiating using the shell.

* Slight improvement around shell like decoding.

* Simplify command extraction.

* Add shared-library category type.

* Fix bug in command extraction.

* Typo in transcendental category.

* Enable unit-test on in smoke test category.

* Make parsing failing output as a failing test.

* Fixes for transcendental tests. Disable tests that do not work.

* Changed category parsing.

* Removed the TestResult parameter from _gatherTestsForFile.
Made testsList only output.

* Remove testing if all tests were disabled.

* Make args of CommandLine always unescaped.

* Add category.

* Don't need escaping on unix/linux.

* Remove some no longer used functions.

11 files changed, 43 insertions, 118 deletions

diff --git a/source/compiler-core/slang-nvrtc-compiler.cpp b/source/compiler-core/slang-nvrtc-compiler.cpp
index 79ece266b..79280e033 100644
--- a/source/compiler-core/slang-nvrtc-compiler.cpp
+++ b/source/compiler-core/slang-nvrtc-compiler.cpp
@@ -828,7 +828,7 @@ SlangResult NVRTCDownstreamCompiler::compile(const CompileOptions& options, RefP
     dstOptions.setCount(cmdLine.m_args.getCount());
     for (Index i = 0; i < cmdLine.m_args.getCount(); ++i)
     {
-        dstOptions[i] = cmdLine.m_args[i].value.getBuffer();
+        dstOptions[i] = cmdLine.m_args[i].getBuffer();
     }
 
     res  = m_nvrtcCompileProgram(program, int(dstOptions.getCount()), dstOptions.getBuffer());
diff --git a/source/core/slang-process-util.h b/source/core/slang-process-util.h
index d47a4793b..af524940c 100644
--- a/source/core/slang-process-util.h
+++ b/source/core/slang-process-util.h
@@ -18,31 +18,12 @@ struct CommandLine
         Filename,                   ///< The executable is set as a filename
     };
 
-    enum class ArgType
-    {
-        Escaped,
-        Unescaped,
-    };
-
-    struct Arg
-    {
-        ArgType type;               ///< How to interpret the argument value
-        String value;               ///< The argument value
-    };
-
         /// Add args - assumed unescaped
-    void addArg(const String& in) { m_args.add(Arg{ArgType::Unescaped, in}); }
+    void addArg(const String& in) { m_args.add(in); }
     void addArgs(const String* args, Int argsCount) { for (Int i = 0; i < argsCount; ++i) addArg(args[i]); }
 
-        /// Add args - all assumed unescaped
-    void addArgs(const Arg* args, Int argCount) { m_args.addRange(args, argCount); }
-
-        /// Add an escaped arg
-    void addEscapedArg(const String& in) { m_args.add(Arg{ArgType::Escaped, in}); }
-    void addEscapedArgs(const String* args, Int argsCount) { for (Int i = 0; i < argsCount; ++i) addEscapedArg(args[i]); }
-
         /// Find the index of an arg which is exact match for slice
-    SLANG_INLINE Index findArgIndex(const UnownedStringSlice& slice) const;
+    SLANG_INLINE Index findArgIndex(const UnownedStringSlice& slice) const { return m_args.indexOf(slice); }
 
         /// Set the executable path
     void setExecutablePath(const String& path) { m_executableType = ExecutableType::Path; m_executable = path; }
@@ -62,7 +43,7 @@ struct CommandLine
 
     ExecutableType m_executableType;    ///< How the executable is specified
     String m_executable;                ///< Executable to run. Note that the executable is never escaped.
-    List<Arg> m_args;                   ///< The arguments  
+    List<String> m_args;                ///< The arguments (Stored *unescaped*)
 };
 
 struct ExecuteResult
@@ -101,42 +82,16 @@ struct ProcessUtil
 };
 
 // -----------------------------------------------------------------------
-SLANG_INLINE Index CommandLine::findArgIndex(const UnownedStringSlice& slice) const
-{
-    const Index count = m_args.getCount();
-
-    for (Index i = 0; i < count; ++i)
-    {
-        const auto& arg = m_args[i];
-        if (arg.value == slice)
-        {
-            return i;
-        }
-    }
-    return -1;
-}
-
-// -----------------------------------------------------------------------
 SLANG_INLINE void CommandLine::addPrefixPathArg(const char* prefix, const String& path, const char* pathPostfix)
 {
-    auto escapeHandler = ProcessUtil::getEscapeHandler();
-
     StringBuilder builder;
-    builder << prefix;
+    builder << prefix << path;
     if (pathPostfix)
     {
         // Work out the path with the postfix
-        StringBuilder fullPath;
-        fullPath << path << pathPostfix;  
-        StringEscapeUtil::appendMaybeQuoted(escapeHandler, fullPath.getUnownedSlice(), builder);
+        builder << pathPostfix;  
     }
-    else
-    {
-        StringEscapeUtil::appendMaybeQuoted(escapeHandler, path.getUnownedSlice(), builder);
-    } 
-
-    // This arg doesn't need subsequent escaping
-    addEscapedArg(builder);
+    addArg(builder.ProduceString());
 }
 
 }
diff --git a/source/core/slang-string-escape-util.cpp b/source/core/slang-string-escape-util.cpp
index 13fce6dc7..5e4db269c 100644
--- a/source/core/slang-string-escape-util.cpp
+++ b/source/core/slang-string-escape-util.cpp
@@ -2,6 +2,7 @@
 
 #include "slang-char-util.h"
 #include "slang-text-io.h"
+#include "slang-memory-arena.h"
 
 #include "../../slang-com-helper.h"
 
@@ -459,12 +460,13 @@ StringEscapeUtil::Handler* StringEscapeUtil::getHandler(Style style)
     }
 }
 
-/* static */void StringEscapeUtil::appendQuoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out)
+/* static */SlangResult StringEscapeUtil::appendQuoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out)
 {
     const char quoteChar = handler->getQuoteChar();
     out.appendChar(quoteChar);
-    handler->appendEscaped(slice, out);
+    SlangResult res = handler->appendEscaped(slice, out);
     out.appendChar(quoteChar);
+    return res;
 }
 
 /* static */SlangResult StringEscapeUtil::appendUnquoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out)
@@ -480,15 +482,16 @@ StringEscapeUtil::Handler* StringEscapeUtil::getHandler(Style style)
     return handler->appendUnescaped(slice.subString(1, len - 2), out);
 }
 
-/* static */void StringEscapeUtil::appendMaybeQuoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out)
+/* static */SlangResult StringEscapeUtil::appendMaybeQuoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out)
 {
     if (handler->isQuotingNeeded(slice))
     {
-        appendQuoted(handler, slice, out);
+        return appendQuoted(handler, slice, out);
     }
     else
     {
         out.append(slice);
+        return SLANG_OK;
     }
 }
 
diff --git a/source/core/slang-string-escape-util.h b/source/core/slang-string-escape-util.h
index 31f781dc6..9dc653df3 100644
--- a/source/core/slang-string-escape-util.h
+++ b/source/core/slang-string-escape-util.h
@@ -2,6 +2,7 @@
 #define SLANG_CORE_STRING_ESCAPE_UTIL_H
 
 #include "slang-string.h"
+#include "slang-list.h"
 
 namespace Slang {
 
@@ -56,7 +57,7 @@ struct StringEscapeUtil
     static Handler* getHandler(Style style);
 
         /// If quoting is needed appends to out quoted
-    static void appendMaybeQuoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out);
+    static SlangResult appendMaybeQuoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out);
 
         /// If the slice appears to be quoted for the style, unquote it, else just append to out
     static SlangResult appendMaybeUnquoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out);
@@ -65,7 +66,7 @@ struct StringEscapeUtil
     static SlangResult appendUnquoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out);
 
         /// Append with quotes (even if not needed)
-    static void appendQuoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out);
+    static SlangResult appendQuoted(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out);
 
         /// Shells can have multiple quoted sections. This function makes a string with out quoting
     static SlangResult unescapeShellLike(Handler* handler, const UnownedStringSlice& slice, StringBuilder& out);
diff --git a/source/core/unix/slang-unix-process-util.cpp b/source/core/unix/slang-unix-process-util.cpp
index a3b66af38..24107fab3 100644
--- a/source/core/unix/slang-unix-process-util.cpp
+++ b/source/core/unix/slang-unix-process-util.cpp
@@ -4,6 +4,7 @@
 #include "../slang-common.h"
 #include "../slang-string-util.h"
 #include "../slang-string-escape-util.h"
+#include "../slang-memory-arena.h"
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -40,20 +41,15 @@ namespace Slang {
     auto escapeHandler = getEscapeHandler();
 
     // When outputting the command line we potentially need to escape the path to the
-    // command and args - that aren't already explicitly marked as escaped. 
+    // command and args - that aren't already explicitly marked as escaped.
+
     StringBuilder cmd;
     StringEscapeUtil::appendMaybeQuoted(escapeHandler, commandLine.m_executable.getUnownedSlice(), cmd);
     for (const auto& arg : commandLine.m_args)
     {
         cmd << " ";
-        if (arg.type == CommandLine::ArgType::Unescaped)
-        {
-            StringEscapeUtil::appendMaybeQuoted(escapeHandler, arg.value.getUnownedSlice(), cmd);
-        }
-        else
-        {
-            cmd << arg.value;
-        }
+
+        StringEscapeUtil::appendMaybeQuoted(escapeHandler, arg.getUnownedSlice(), cmd);
     }
     return cmd.ToString();
 }
@@ -63,16 +59,17 @@ namespace Slang {
     outExecuteResult.init();
     
     List<char const*> argPtrs;
+
     // Add the command
     argPtrs.add(commandLine.m_executable.getBuffer());
 
-    // Add all the args - they don't need any explicit escaping
+    // Add all the args - they don't need any explicit escaping 
     for (auto arg : commandLine.m_args)
     {
-        // All args for this target must be unescaped
-        SLANG_ASSERT(arg.type == CommandLine::ArgType::Unescaped);
-        argPtrs.add(arg.value.getBuffer());
+        // All args for this target must be unescaped (as they are in CommandLine)
+        argPtrs.add(arg.getBuffer());
     }
+
     // Terminate with a null
     argPtrs.add(nullptr);
 
diff --git a/source/core/windows/slang-win-process-util.cpp b/source/core/windows/slang-win-process-util.cpp
index 9cc567318..4039361a4 100644
--- a/source/core/windows/slang-win-process-util.cpp
+++ b/source/core/windows/slang-win-process-util.cpp
@@ -169,14 +169,7 @@ static DWORD WINAPI _readerThreadProc(LPVOID threadParam)
     for (const auto& arg : commandLine.m_args)
     {
         cmd << " ";
-        if (arg.type == CommandLine::ArgType::Unescaped)
-        {
-            StringEscapeUtil::appendMaybeQuoted(escapeHandler, arg.value.getUnownedSlice(), cmd);
-        }
-        else
-        {
-            cmd << arg.value;
-        }
+        StringEscapeUtil::appendMaybeQuoted(escapeHandler, arg.getUnownedSlice(), cmd);
     }
     return cmd.ToString();
 }
diff --git a/tools/render-test/options.cpp b/tools/render-test/options.cpp
index b5d75adf2..dcf0b77b1 100644
--- a/tools/render-test/options.cpp
+++ b/tools/render-test/options.cpp
@@ -168,10 +168,8 @@ static SlangResult _setRendererType(DeviceType type, const char* arg, Slang::Wri
                 return SLANG_FAIL;
             }
 
-            CommandLine::Arg arg;
-            arg.type = CommandLine::ArgType::Escaped;
-            arg.value = *argCursor++;
-            outOptions.compileArgs.add(arg);
+            const char* compileArg = *argCursor++;
+            outOptions.compileArgs.add(compileArg);
         }
         else if (strcmp(arg, "-performance-profile") == 0)
         {
diff --git a/tools/render-test/options.h b/tools/render-test/options.h
index 6ca1ef499..2945113fe 100644
--- a/tools/render-test/options.h
+++ b/tools/render-test/options.h
@@ -73,7 +73,7 @@ struct Options
 
     Slang::List<Slang::String> renderFeatures;          /// Required render features for this test to run
 
-    Slang::List<Slang::CommandLine::Arg> compileArgs;
+    Slang::List<Slang::String> compileArgs;
 
     Slang::String adapter;                              ///< The adapter to use either name or index
 
diff --git a/tools/render-test/slang-support.cpp b/tools/render-test/slang-support.cpp
index 840b0d3e2..5470ea35a 100644
--- a/tools/render-test/slang-support.cpp
+++ b/tools/render-test/slang-support.cpp
@@ -93,7 +93,7 @@ void ShaderCompilerUtil::Output::reset()
         List<const char*> args;
         for (const auto& arg : request.compileArgs)
         {
-            args.add(arg.value.getBuffer());
+            args.add(arg.getBuffer());
         }
         SLANG_RETURN_ON_FAIL(spProcessCommandLineArguments(slangRequest, args.getBuffer(), int(args.getCount())));
     }
diff --git a/tools/render-test/slang-support.h b/tools/render-test/slang-support.h
index 7e00a2c72..59d422bc7 100644
--- a/tools/render-test/slang-support.h
+++ b/tools/render-test/slang-support.h
@@ -39,7 +39,7 @@ struct ShaderCompileRequest
     Slang::List<Slang::String> globalSpecializationArgs;
     Slang::List<Slang::String> entryPointSpecializationArgs;
 
-    Slang::List<Slang::CommandLine::Arg> compileArgs;
+    Slang::List<Slang::String> compileArgs;
 };
 
 
diff --git a/tools/slang-test/slang-test-main.cpp b/tools/slang-test/slang-test-main.cpp
index 3a9b15648..530d9a115 100644
--- a/tools/slang-test/slang-test-main.cpp
+++ b/tools/slang-test/slang-test-main.cpp
@@ -553,24 +553,6 @@ Result spawnAndWaitExe(TestContext* context, const String& testPath, const Comma
     return res;
 }
 
-static const char* _getUnescaped(StringEscapeHandler* handler, const CommandLine::Arg& arg, MemoryArena& arena)
-{
-    if (arg.type == CommandLine::ArgType::Escaped)
-    {
-        StringBuilder buf;
-        StringEscapeUtil::unescapeShellLike(handler, arg.value.getUnownedSlice(), buf);
-
-        // We strictly only need to allocate if the result is different.
-        // That an arg marked as 'escaped' does not mean it produces a different result when decoding.
-        if (buf != arg.value)
-        {
-            return arena.allocateString(buf.getBuffer(), buf.getLength());
-        } 
-    }
-
-    return arg.value.getBuffer();
-}
-
 Result spawnAndWaitSharedLibrary(TestContext* context, const String& testPath, const CommandLine& cmdLine, ExecuteResult& outRes)
 {
     const auto& options = context->options;
@@ -618,19 +600,11 @@ Result spawnAndWaitSharedLibrary(TestContext* context, const String& testPath, c
 
         String exePath = Path::combine(context->exeDirectoryPath, exeName);
 
-        
-        // Use the arena to hold any unescaped strings
-        MemoryArena arena(1024);
         List<const char*> args;
-
         args.add(exePath.getBuffer());
-
+        for (const auto& cmdArg : cmdLine.m_args)
         {
-            auto escapeHandler = ProcessUtil::getEscapeHandler();
-            for (Index i = 0; i < cmdLine.m_args.getCount(); ++i)
-            {
-                args.add(_getUnescaped(escapeHandler, cmdLine.m_args[i], arena));
-            }
+            args.add(cmdArg.getBuffer());
         }
 
         SlangResult res = func(&stdWriters, context->getSession(), int(args.getCount()), args.begin());
@@ -656,7 +630,7 @@ static SlangResult _extractArg(const CommandLine& cmdLine, const String& argName
 
     if (index >= 0 && index < cmdLine.getArgCount() - 1)
     {
-        outValue = cmdLine.m_args[index + 1].value;
+        outValue = cmdLine.m_args[index + 1];
         return SLANG_OK;
     }
     return SLANG_FAIL;
@@ -738,7 +712,7 @@ static SlangResult _extractRenderTestRequirements(const CommandLine& cmdLine, Te
 
         for (const auto& arg: args)
         {
-            Slang::UnownedStringSlice argSlice = arg.value.getUnownedSlice();
+            Slang::UnownedStringSlice argSlice = arg.getUnownedSlice();
             if (argSlice.getLength() && argSlice[0] == '-')
             {
                 // Look up the rendering API if set
@@ -1438,12 +1412,16 @@ TestResult runCompile(TestContext* context, TestInput& input)
     CommandLine cmdLine;
     _initSlangCompiler(context, cmdLine);
 
+    StringEscapeHandler* escapeHandler = StringEscapeUtil::getHandler(StringEscapeUtil::Style::Space);
+
     for (auto arg : input.testOptions->args)
     {
         // If there is a quote in the string, assume it is 'escaped'. 
-        if (arg.indexOf('"') >= 0)
+        if (arg.indexOf(escapeHandler->getQuoteChar()) >= 0)
         {
-            cmdLine.addEscapedArg(arg);
+            StringBuilder buf;
+            StringEscapeUtil::unescapeShellLike(escapeHandler, arg.getUnownedSlice(), buf);
+            cmdLine.addArg(buf.ProduceString());
         }
         else
         {