Improved command line control for apis and synthesized tests (#616)

* Parsing of control of api parameters no longer needs comma separator.
Parsing of API list now can take an initial state.
Document the command line option.

* * Proper handling of 'default' (or initialFlags) - by using if the first token is an operator.
* Clarified parsing of api flags.
* Now 'vk' will mean just use vk. +vk will mean the defaults plus vk, and -vk is the defaults -vk.

* Improve README.md on api expressions.
Improve error text for failure to parse api expressions.

4 files changed, 169 insertions, 45 deletions

diff --git a/tools/slang-test/README.md b/tools/slang-test/README.md
index 819694b48..1dc44a8b8 100644
--- a/tools/slang-test/README.md
+++ b/tools/slang-test/README.md
@@ -16,6 +16,32 @@ slang-test -bindir E:\slang\bin\windows-x64\Debug\\ -category full tests/compute
 * The -category full means that all tests can be run.
 * The final 'free parameter' is 'tests/compute/array-param' and means only tests starting with this string will run.
 
+The command line can control which tests are run with a couple of switches
+
+* -api - Overall controls over which apis will be tested against 
+* -synthesizedTestApi - Controls which apis will have tests synthesized for them from other apis. Tests can be synthesized for dx12 and vulkan.
+
+The parameter afterwards is an 'api expression' used to control which apis will or wont be used. This is somewhat like a mathematical expression with only + and - operations and api names. If the first operation is + or - it will be applied to whatever the default is, otherwise the defaults are ignored.
+
+* vk - just for vulkan
+* +vk - Whatever the defaults are including vulkan
+* -dx12 - Whatever the defaults are excluding vulkan
+* all - for all apis, none - for no apis
+* all-vk - all apis but not vulkan (vk)
+* all-vk-dx12 - all apis but not vulkan (vk) or directx12 (dx12)
+* gl+dx11 - just on opengl (gl) and directx11 (dx11)
+* +none - same as defaults 
+
+So if you wanted to test for all apis, except opengl you'd put on the command line '-api all-gl'
+
+The different APIs are 
+
+* OpenGL - gl,ogl,opengl
+* Vulkan - vk,vulkan
+* DirectD3D12 - dx12,d3d12
+* DirectD3D11 -dx11,d3d11
+
+
 It may also be necessary to have the working directory the root directory of the slang distribution - in the example above this would be "E:\slang\". 
 
 ## Test Categories
diff --git a/tools/slang-test/main.cpp b/tools/slang-test/main.cpp
index 12434301c..49279e81c 100644
--- a/tools/slang-test/main.cpp
+++ b/tools/slang-test/main.cpp
@@ -199,7 +199,11 @@ struct Options
     Dictionary<TestCategory*, TestCategory*> excludeCategories;
 
     // By default we can test against all apis
-    int enabledApis = int(RenderApiFlag::AllOf);
+    RenderApiFlags enabledApis = RenderApiFlag::AllOf;
+
+    // By default we potentially synthesize test for all 
+    // TODO: Vulkan is disabled by default for now as the majority as vulkan synthesized tests fail  
+    RenderApiFlags synthesizedTestApis = RenderApiFlag::AllOf & ~RenderApiFlag::Vulkan;
 };
 
 // Globals
@@ -605,15 +609,31 @@ Result parseOptions(int* argc, char** argv)
         {
             if (argCursor == argEnd)
             {
-                fprintf(stderr, "error: expected comma separated list of apis '%s'\n", arg);
+                fprintf(stderr, "error: expecting an api expression (eg 'vk+dx12' or '+dx11') '%s'\n", arg);
+                return SLANG_FAIL;
+            }
+            const char* apiList = *argCursor++;
+
+            SlangResult res = RenderApiUtil::parseApiFlags(UnownedStringSlice(apiList), g_options.enabledApis, &g_options.enabledApis);
+            if (SLANG_FAILED(res))
+            {
+                fprintf(stderr, "error: unable to parse api expression '%s'\n", apiList);
+                return res;
+            }
+        }
+        else if (strcmp(arg, "-synthesizedTestApi") == 0)
+        {
+            if (argCursor == argEnd)
+            {
+                fprintf(stderr, "error: expected an api expression (eg 'vk+dx12' or '+dx11') '%s'\n", arg);
                 return SLANG_FAIL;
             }
             const char* apiList = *argCursor++;
 
-            SlangResult res = RenderApiUtil::parseApiFlags(UnownedStringSlice(apiList), &g_options.enabledApis);
+            SlangResult res = RenderApiUtil::parseApiFlags(UnownedStringSlice(apiList), g_options.synthesizedTestApis, &g_options.synthesizedTestApis);
             if (SLANG_FAILED(res))
             {
-                fprintf(stderr, "error: unable to parse api list '%s'\n", apiList);
+                fprintf(stderr, "error: unable to parse api expression '%s'\n", apiList);
                 return res;
             }
         }
@@ -629,6 +649,9 @@ Result parseOptions(int* argc, char** argv)
         const int availableApis = RenderApiUtil::getAvailableApis();
         // Only allow apis we know are available
         g_options.enabledApis &= availableApis;
+
+        // Can only synth for apis that are available
+        g_options.synthesizedTestApis &= g_options.enabledApis;
     }
 
     // any arguments left over were positional arguments
@@ -2066,7 +2089,7 @@ void runTestsOnFile(
     List<TestOptions> synthesizedTests;
 
     // If dx12 is available synthesize Dx12 test
-    if ((g_options.enabledApis & RenderApiFlag::D3D12) != 0)
+    if ((g_options.synthesizedTestApis & RenderApiFlag::D3D12) != 0)
     {
         // If doesn't have option generate dx12 options from dx11
         if (!hasRenderOption(RenderApiType::D3D12, testList))
@@ -2088,9 +2111,8 @@ void runTestsOnFile(
         }
     }
 
-#if 0
     // If Vulkan is available synthesize Vulkan test
-    if ((g_options.enabledApis & RenderApiFlag::Vulkan) != 0)
+    if ((g_options.synthesizedTestApis & RenderApiFlag::Vulkan) != 0)
     {
         // If doesn't have option generate dx12 options from dx11
         if (!hasRenderOption(RenderApiType::Vulkan, testList))
@@ -2102,7 +2124,7 @@ void runTestsOnFile(
                 // If it's a render test, and there is on d3d option, add one
                 if (isRenderTest(testOptions.command) && !isHLSLTest(testOptions.command) && !hasRenderOption(RenderApiType::Vulkan, testOptions))
                 {
-                    // Add with -dx12 option
+                    // Add with -vk option
                     TestOptions testOptionsCopy(testOptions);
                     testOptionsCopy.args.Add("-vk");
 
@@ -2112,13 +2134,11 @@ void runTestsOnFile(
                         testOptionsCopy.args.RemoveAt(index);
                     }
 
-
                     synthesizedTests.Add(testOptionsCopy);
                 }
             }
         }
     }
-#endif
 
     // Add any tests that were synthesized
     for (UInt i = 0; i < synthesizedTests.Count(); ++i)
diff --git a/tools/slang-test/render-api-util.cpp b/tools/slang-test/render-api-util.cpp
index 5f47f3b49..06a915df8 100644
--- a/tools/slang-test/render-api-util.cpp
+++ b/tools/slang-test/render-api-util.cpp
@@ -59,66 +59,142 @@ static int _calcAvailableApis()
     return RenderApiType::Unknown;
 }
 
-/* static */int RenderApiUtil::findApiFlagsByName(const Slang::UnownedStringSlice& name)
+/* static */ Slang::Result RenderApiUtil::findApiFlagsByName(const Slang::UnownedStringSlice& name, RenderApiFlags* flagsOut)
 {
     // Special case 'all'
     if (name == "all")
     {
-        return int(RenderApiFlag::AllOf);
+        *flagsOut = RenderApiFlags(RenderApiFlag::AllOf);
+        return SLANG_OK;
+    }
+    if (name == "none")
+    {
+        *flagsOut = RenderApiFlags(0);
+        return SLANG_OK;
     }
     RenderApiType type = findApiTypeByName(name);
-    return (type == RenderApiType::Unknown) ? 0 : (1 << int(type));
+    if (type == RenderApiType::Unknown)
+    {
+        return SLANG_FAIL;
+    }
+    *flagsOut = RenderApiFlags(1) << int(type);
+    return SLANG_OK;
 }
 
-/* static */Slang::Result RenderApiUtil::parseApiFlags(const Slang::UnownedStringSlice& text, int* apiBitsOut)
+static bool isNameStartChar(char c)
+{
+    return (c >= 'a' && c <='z') || (c >= 'A' && c <= 'Z') || (c == '_');
+}
+
+static bool isNameNextChar(char c)
+{
+    return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c == '_') || (c >= '0' && c <= '9');
+}
+
+namespace { // anonymous
+enum class Token 
+{
+    eError,
+    eOp,
+    eId,
+    eEnd,
+};
+}
+
+static Token nextToken(Slang::UnownedStringSlice& textInOut, Slang::UnownedStringSlice& lexemeOut)
 {
     using namespace Slang;
+    if (textInOut.size() <= 0)
+    {
+        return Token::eEnd;
+    }
+    const char* start = textInOut.begin();
+    const char* end = textInOut.end();
+
+    const char firstChar = start[0];
+    if (firstChar == '-' || firstChar == '+')
+    {
+        lexemeOut = UnownedStringSlice(start, start + 1);
+        textInOut = UnownedStringSlice(start + 1, end);
+        return Token::eOp;
+    }
+
+    if (!isNameStartChar(firstChar))
+    {
+        lexemeOut = UnownedStringSlice(start, start + 1);
+        return Token::eError;
+    }
+    const char* cur = start + 1;
+    while (cur < end && isNameNextChar(*cur))
+    {
+        cur++;
+    }
 
-    int apiBits = 0;
+    lexemeOut = UnownedStringSlice(start, cur);
+    textInOut = UnownedStringSlice(cur, end);
+    return Token::eId;
+}
 
-    List<UnownedStringSlice> slices;
-    StringUtil::split(text, ',', slices);
+/* static */Slang::Result RenderApiUtil::parseApiFlags(const Slang::UnownedStringSlice& textIn, RenderApiFlags initialFlags, RenderApiFlags* apiFlagsOut)
+{
+    using namespace Slang;
 
-    for (int i = 0; i < int(slices.Count()); ++i)
+    UnownedStringSlice text(textIn);
+    UnownedStringSlice lexeme;
+
+    RenderApiFlags apiFlags = 0; 
+    
+    switch (nextToken(text, lexeme))
     {
-        UnownedStringSlice slice = slices[i];
-        bool add = true;
-        if (slice.size() <= 0)
+        case Token::eOp:
         {
-            return SLANG_FAIL;
+            // If we start with an op - we use the passed in values as the default
+            // Rewind back to the start
+            text = textIn;
+            apiFlags = initialFlags;
+            break;
         }
-        if (slice[0] == '+')
+        case Token::eId:
         {
-            // Drop the +
-            slice = UnownedStringSlice(slice.begin() + 1, slice.end());
+            // If we start with an Id - we use that as the starting state
+            SLANG_RETURN_ON_FAIL(findApiFlagsByName(lexeme, &apiFlags));
+            break;
         }
-        else if (slice[0] == '-')
+        default: return SLANG_FAIL;
+    }
+    
+    while (true)
+    {
+        // Must have an op followed by an id unless we are at the end
+        switch (nextToken(text, lexeme))
         {
-            add = false;
-            // Drop the +
-            slice = UnownedStringSlice(slice.begin() + 1, slice.end());
+            case Token::eEnd:
+            {
+                *apiFlagsOut = apiFlags;
+                return SLANG_OK;
+            }
+            case Token::eOp:    break;
+            default:            return SLANG_FAIL;
         }
 
-        // We need to find the bits... 
-        int bits = findApiFlagsByName(slice);
-        // 0 means an error
-        if (bits == 0)
+        const char op = lexeme[0];
+        if (nextToken(text, lexeme) != Token::eId)
         {
             return SLANG_FAIL;
         }
 
-        if (add)
+        RenderApiFlags flags;
+        SLANG_RETURN_ON_FAIL(findApiFlagsByName(lexeme, &flags));
+
+        if (op == '+')
         {
-            apiBits |= bits;
+            apiFlags |= flags;
         }
         else
         {
-            apiBits &= ~bits;
+            apiFlags &= ~flags;
         }
     }
-
-    *apiBitsOut = apiBits;
-    return SLANG_OK;
 }
 
 /* !!!!!!!!!!!!!!!!!!!!!!!!!!!! Platform specific stuff !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
diff --git a/tools/slang-test/render-api-util.h b/tools/slang-test/render-api-util.h
index a44528faf..1c3c81254 100644
--- a/tools/slang-test/render-api-util.h
+++ b/tools/slang-test/render-api-util.h
@@ -43,14 +43,16 @@ struct RenderApiUtil
         /// Returns a combination of RenderApiFlag bits which if set indicates that the API is available.
     static int getAvailableApis();
 
-        /// Returns -1 if unknown
+        /// Returns RenderApiType::Unknown if not found
     static RenderApiType findApiTypeByName(const Slang::UnownedStringSlice& name);
-        /// Returns 0 if none found.
-    static int findApiFlagsByName(const Slang::UnownedStringSlice& name);
+        /// FlagsOut will have flag/flags specified by a name if returns with successful result code.
+    static Slang::Result findApiFlagsByName(const Slang::UnownedStringSlice& name, RenderApiFlags* flagsOut);
 
-        /// Parse api flags string (comma delimited list of api names, or 'all' for all)
-        /// For example "all,-dx12" would be all apis, except dx12
-    static Slang::Result parseApiFlags(const Slang::UnownedStringSlice& text, int* apiBitsOut);
+        /// Parse api flags string, returning SLANG_OK on success.
+        /// If first character is + or - the flags will be applied to initialFlags, else initialFlags is ignored.
+        /// For example "all-dx12" would be all apis, except dx12
+        /// -vk would be whatever is in initial flags, but not vulkan.
+    static Slang::Result parseApiFlags(const Slang::UnownedStringSlice& text, RenderApiFlags initialFlags, RenderApiFlags* apiBitsOut);
 
         /// Get information about a render API
     static const Info& getInfo(RenderApiType type) { return s_infos[int(type)]; }