Add `_Internal`/`External` atom enforcement and validation. (#4702)

* Add `_Internal`/`External` atom validation and use enforcement.

Fixes: #4676

Changes:
* Added `validateInternalAtomExternalAtomPair` to the capability generator to ensure all `_Internal` atoms have a corresponding `External` atom.
* Validation of 'RequireCapabilityAttribute' warns if a user uses an '_Internal' atom.
* Added 'External' atoms to atoms with an already existing '_Internal' atom.
* Printing an atom removes '_'.
* Fixed some incorrect which were checking for the incorrect warning/error (capability4.slang, capability5.slang, capability6.slang).

* switch capability name to use `UnownedStringSlice` instead of `const char*`

switch capability name to use `UnownedStringSlice` instead of `const char*`, this includes using functions like `.startsWith`.

* grammer

---------

Co-authored-by: Yong He <yonghe@outlook.com>

11 files changed, 111 insertions, 30 deletions

diff --git a/source/slang/slang-capabilities.capdef b/source/slang/slang-capabilities.capdef
index 59e79a07a..264748738 100644
--- a/source/slang/slang-capabilities.capdef
+++ b/source/slang/slang-capabilities.capdef
@@ -372,6 +372,8 @@ alias GL_ARB_shader_image_load_store = GL_EXT_shader_image_load_store;
 alias GL_ARB_shader_image_size = _GL_ARB_shader_image_size | spvImageQuery | metal;
 alias GL_ARB_texture_multisample = _GL_ARB_texture_multisample | _spirv_1_0;
 alias GL_ARB_shader_texture_image_samples = _GL_ARB_shader_texture_image_samples | spvImageQuery | metal;
+alias GL_ARB_sparse_texture = _GL_ARB_sparse_texture | spvSparseResidency;
+alias GL_ARB_sparse_texture2 = _GL_ARB_sparse_texture2 | spvSparseResidency;
 alias GL_ARB_sparse_texture_clamp = _GL_ARB_sparse_texture_clamp | spvSparseResidency;
 alias GL_ARB_texture_gather = _GL_ARB_texture_gather | spvImageGatherExtended | metal;
 alias GL_ARB_texture_query_levels = _GL_ARB_texture_query_levels | spvImageQuery | metal;
@@ -425,6 +427,8 @@ alias bufferreference_int64 = bufferreference + GL_EXT_shader_explicit_arithmeti
 
 // Define what each shader model means on different targets.
 
+// spirv profile
+
 alias spirv_1_0 = _spirv_1_0;
 alias spirv_1_1 = _spirv_1_1
                 | spirv_1_0
@@ -446,6 +450,16 @@ alias spirv_1_6 = _spirv_1_6 + GL_EXT_debug_printf + GL_EXT_demote_to_helper_inv
                 ;
 alias spirv_latest = _spirv_1_6;
 
+alias SPIRV_1_0 = spirv_1_0;
+alias SPIRV_1_1 = spirv_1_1;
+alias SPIRV_1_2 = spirv_1_2;
+alias SPIRV_1_3 = spirv_1_3;
+alias SPIRV_1_4 = spirv_1_4;
+alias SPIRV_1_5 = spirv_1_5;
+alias SPIRV_1_6 = spirv_1_6;
+
+// hlsl profile
+
 alias sm_4_0_version = _sm_4_0
              | _GLSL_150
              | spirv_1_0
@@ -597,7 +611,21 @@ alias sm_6_7_version = _sm_6_7
 alias sm_6_7 = sm_6_7_version
              | sm_6_6
              ;
-// Profiles
+
+alias DX_4_0 = sm_4_0;
+alias DX_4_1 = sm_4_1;
+alias DX_5_0 = sm_5_0;
+alias DX_5_1 = sm_5_1;
+alias DX_6_0 = sm_6_0;
+alias DX_6_1 = sm_6_1;
+alias DX_6_2 = sm_6_2;
+alias DX_6_3 = sm_6_3;
+alias DX_6_4 = sm_6_4;
+alias DX_6_5 = sm_6_5;
+alias DX_6_6 = sm_6_6;
+alias DX_6_7 = sm_6_7;
+
+// glsl profile
 
 alias GLSL_130 = _GLSL_130
                | _sm_4_0
@@ -711,30 +739,27 @@ alias GLSL_410_SPIRV_1_0 = _GLSL_410 | spirv_1_0;
 alias GLSL_420_SPIRV_1_0 = _GLSL_420 + GLSL_410_SPIRV_1_0 | GLSL_410_SPIRV_1_0;
 alias GLSL_430_SPIRV_1_0 = _GLSL_430 + GLSL_420_SPIRV_1_0 | GLSL_420_SPIRV_1_0;
 
-alias DX_4_0 = sm_4_0;
-alias DX_4_1 = sm_4_1;
-alias DX_5_0 = sm_5_0;
-alias DX_5_1 = sm_5_1;
-alias DX_6_0 = sm_6_0;
-alias DX_6_1 = sm_6_1;
-alias DX_6_2 = sm_6_2;
-alias DX_6_3 = sm_6_3;
-alias DX_6_4 = sm_6_4;
-alias DX_6_5 = sm_6_5;
-alias DX_6_6 = sm_6_6;
-alias DX_6_7 = sm_6_7;
+// cuda profile
 
+alias cuda_sm_1_0 = _cuda_sm_1_0 | sm_4_0;
+alias cuda_sm_2_0 = _cuda_sm_2_0 | sm_4_1;
+alias cuda_sm_3_0 = _cuda_sm_3_0 | sm_6_0;
+alias cuda_sm_3_5 = _cuda_sm_3_5 | sm_6_0;
+alias cuda_sm_4_0 = _cuda_sm_4_0 | sm_6_0;
+alias cuda_sm_5_0 = _cuda_sm_5_0 | sm_6_0;
+alias cuda_sm_6_0 = _cuda_sm_6_0 | sm_6_0;
+alias cuda_sm_7_0 = _cuda_sm_7_0 | sm_5_1;
+alias cuda_sm_8_0 = _cuda_sm_8_0 | sm_5_1;
+alias cuda_sm_9_0 = _cuda_sm_9_0 | sm_5_1;
+
+// metal profile
 
 alias METAL_2_3 = metallib_2_3;
 alias METAL_2_4 = metallib_2_4;
+alias METAL_3_0 = metallib_3_0;
+alias METAL_3_1 = metallib_3_1;
 
-alias SPIRV_1_0 = spirv_1_0;
-alias SPIRV_1_1 = spirv_1_1;
-alias SPIRV_1_2 = spirv_1_2;
-alias SPIRV_1_3 = spirv_1_3;
-alias SPIRV_1_4 = spirv_1_4;
-alias SPIRV_1_5 = spirv_1_5;
-alias SPIRV_1_6 = spirv_1_6;
+// Profiles of convenience
 
 alias appendstructuredbuffer = sm_5_0_version;
 alias atomic_hlsl = _sm_4_0;
diff --git a/source/slang/slang-capability.cpp b/source/slang/slang-capability.cpp
index f06ccb663..00a3b6001 100644
--- a/source/slang/slang-capability.cpp
+++ b/source/slang/slang-capability.cpp
@@ -49,7 +49,7 @@ enum class CapabilityNameFlavor : int32_t
 struct CapabilityAtomInfo
 {
     /// The API-/language-exposed name of the capability.
-    char const* name;
+    UnownedStringSlice name;
 
     /// Flavor of atom: concrete, abstract, or alias
     CapabilityNameFlavor        flavor;
@@ -85,14 +85,14 @@ void getCapabilityNames(List<UnownedStringSlice>& ioNames)
     {
         if (_getInfo(CapabilityName(i)).flavor != CapabilityNameFlavor::Abstract)
         {
-            ioNames.add(UnownedStringSlice(_getInfo(CapabilityName(i)).name));
+            ioNames.add(_getInfo(CapabilityName(i)).name);
         }
     }
 }
 
 UnownedStringSlice capabilityNameToString(CapabilityName name)
 {
-    return UnownedStringSlice(_getInfo(name).name);
+    return _getInfo(name).name;
 }
 
 bool isDirectChildOfAbstractAtom(CapabilityAtom name)
@@ -111,7 +111,7 @@ bool isTargetVersionAtom(CapabilityAtom name)
 
 bool isSpirvExtensionAtom(CapabilityAtom name)
 {
-    return UnownedStringSlice(_getInfo(name).name).startsWith("SPV_");
+    return _getInfo(name).name.startsWith("SPV_");
 }
 
 bool lookupCapabilityName(const UnownedStringSlice& str, CapabilityName& value);
@@ -124,6 +124,12 @@ CapabilityName findCapabilityName(UnownedStringSlice const& name)
     return result;
 }
 
+bool isInternalCapabilityName(CapabilityName name)
+{
+    SLANG_ASSERT(_getInfo(name).name != nullptr);
+    return _getInfo(name).name.startsWith("_");
+}
+
 CapabilityAtom getLatestSpirvAtom()
 {
     static CapabilityAtom result = CapabilityAtom::Invalid;
@@ -964,7 +970,7 @@ void printDiagnosticArg(StringBuilder& sb, const CapabilityAtomSet atomSet)
         CapabilityName formattedAtom = (CapabilityName)atom;
         if (!isFirst)
             sb << " + ";
-        sb << capabilityNameToStringWithoutPrefix(formattedAtom);
+        printDiagnosticArg(sb, formattedAtom);
         isFirst = false;
     }
 }
diff --git a/source/slang/slang-capability.h b/source/slang/slang-capability.h
index 79b1f7068..2e12af622 100644
--- a/source/slang/slang-capability.h
+++ b/source/slang/slang-capability.h
@@ -340,6 +340,9 @@ bool isCapabilityDerivedFrom(CapabilityAtom atom, CapabilityAtom base);
     /// Find a capability atom with the given `name`, or return CapabilityAtom::Invalid.
 CapabilityName findCapabilityName(UnownedStringSlice const& name);
 
+    /// Check if 'name' is an '_Internal' or 'External' capability.
+bool isInternalCapabilityName(CapabilityName name);
+
 CapabilityAtom getLatestSpirvAtom();
 CapabilityAtom getLatestMetalAtom();
 
diff --git a/source/slang/slang-check-modifier.cpp b/source/slang/slang-check-modifier.cpp
index e91d2e45b..483ff0e18 100644
--- a/source/slang/slang-check-modifier.cpp
+++ b/source/slang/slang-check-modifier.cpp
@@ -929,6 +929,8 @@ namespace Slang
                 if (checkCapabilityName(arg, capName))
                 {
                     capabilityNames.add(capName);
+                    if(isInternalCapabilityName(capName))
+                        maybeDiagnose(getSink(), this->getOptionSet(), DiagnosticCategory::Capability, attr, Diagnostics::usingInternalCapabilityName, attr, capName);
                 }
             }
             requireCapAttr->capabilitySet = CapabilitySet(capabilityNames);
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index bf478a17d..fd7bd4a65 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -395,6 +395,7 @@ DIAGNOSTIC(36109, Error, invalidTargetSwitchCase, "'$0' cannot be used as a targ
 DIAGNOSTIC(36110, Error, stageIsIncompatibleWithCapabilityDefinition, "'$0' is defined for stage '$1', which is incompatible with the declared capability set '$2'.")
 DIAGNOSTIC(36111, Error, unexpectedCapability, "'$0' resolves into a disallowed `$1` Capability.")
 DIAGNOSTIC(36112, Warning, entryPointAndProfileAreIncompatible, "'$0' is defined for stage '$1', which is incompatible with the declared profile '$2'.")
+DIAGNOSTIC(36113, Warning, usingInternalCapabilityName, "'$0' resolves into a '_Internal' `_$1' Capability, use '$1' instead.")
 
 // Attributes
 DIAGNOSTIC(31000, Warning, unknownAttributeName, "unknown attribute '$0'")
diff --git a/tests/language-feature/capability/capability4.slang b/tests/language-feature/capability/capability4.slang
index 4a2a6f3c9..eda9a86be 100644
--- a/tests/language-feature/capability/capability4.slang
+++ b/tests/language-feature/capability/capability4.slang
@@ -1,6 +1,6 @@
 //TEST:SIMPLE(filecheck=CHECK): -target spirv -emit-spirv-directly -entry main -stage compute
 //TEST:SIMPLE(filecheck=CHECK_IGNORE_CAPS): -target spirv -emit-spirv-directly -entry main -stage compute -ignore-capabilities
-// CHECK_IGNORE_CAPS-NOT: error 36104
+// CHECK_IGNORE_CAPS-NOT: error 36108
 
 // Check that a non-static member method implictly requires capabilities
 // defined in ThisType.
diff --git a/tests/language-feature/capability/capability5.slang b/tests/language-feature/capability/capability5.slang
index 6199f045a..33a002175 100644
--- a/tests/language-feature/capability/capability5.slang
+++ b/tests/language-feature/capability/capability5.slang
@@ -3,7 +3,7 @@
 //TEST:SIMPLE(filecheck=PASS): -target glsl -entry main -stage compute -allow-glsl -profile sm_6_0
 //TEST:SIMPLE(filecheck=WARN): -target glsl -entry main -stage compute -allow-glsl -capability GLSL_130
 //TEST:SIMPLE(filecheck=ERROR): -target glsl -entry main -stage compute -allow-glsl -capability GLSL_130 -restrictive-capability-check
-// CHECK_IGNORE_CAPS-NOT: error 36104
+// CHECK_IGNORE_CAPS-NOT: error 41012
 
 // Check that a non-static member method implictly requires capabilities
 // defined in ThisType.
diff --git a/tests/language-feature/capability/capability6.slang b/tests/language-feature/capability/capability6.slang
index 3d6642063..e4347100a 100644
--- a/tests/language-feature/capability/capability6.slang
+++ b/tests/language-feature/capability/capability6.slang
@@ -1,6 +1,6 @@
 //TEST:SIMPLE(filecheck=CHECK): -target spirv -emit-spirv-directly -entry computeMain -stage compute
 //TEST:SIMPLE(filecheck=CHECK_IGNORE_CAPS): -target spirv -emit-spirv-directly -entry computeMain -stage compute -ignore-capabilities
-// CHECK_IGNORE_CAPS-NOT: error 36104
+// CHECK_IGNORE_CAPS-NOT: error 36111
 
 
 
diff --git a/tests/language-feature/capability/usingInternalCapabilityWarning.slang b/tests/language-feature/capability/usingInternalCapabilityWarning.slang
new file mode 100644
index 000000000..48a573ab6
--- /dev/null
+++ b/tests/language-feature/capability/usingInternalCapabilityWarning.slang
@@ -0,0 +1,9 @@
+//TEST:SIMPLE(filecheck=CHECK): -target spirv -emit-spirv-directly -entry main -stage compute
+//TEST:SIMPLE(filecheck=CHECK_IGNORE_CAPS): -target spirv -emit-spirv-directly -entry main -stage compute -ignore-capabilities
+// CHECK_IGNORE_CAPS-NOT: warning 36113
+// CHECK: .slang(5):{{.*}}warning 36113: {{.*}}_GL_NV_ray_tracing_motion_blur{{.*}}GL_NV_ray_tracing_motion_blur
+[require(_GL_NV_ray_tracing_motion_blur)]
+[numthreads(1,1,1)]
+void computeMain()
+{
+}
\ No newline at end of file
diff --git a/tools/slang-capability-generator/capability-generator-main.cpp b/tools/slang-capability-generator/capability-generator-main.cpp
index e4a4885c6..711532273 100644
--- a/tools/slang-capability-generator/capability-generator-main.cpp
+++ b/tools/slang-capability-generator/capability-generator-main.cpp
@@ -258,6 +258,39 @@ struct CapabilityDefParser
         return SLANG_OK;
     }
 
+    void validateInternalAtomExternalAtomPair()
+    {
+        // All `_Internal` atoms must have an `External` atom. 
+        // `External` atoms do not require to have an `_Internal` atom.
+        // The following behavior ensures that if we error with 'atom' instead of 
+        // '_atom' a user may add the 'atom' capability to solve their error. This is
+        // important because '_Internal' will only be for 1 target, 'External' will alias
+        // to more than 1 target. We need to ensure users avoid 'Internal' when possible.
+
+        Dictionary<String, List<RefPtr<CapabilityDef>>> nameToInternalAndExternalAtom;
+        for(auto i : m_defs)
+        {
+            // 'abstract' atoms are not reported to a user and are ignored
+            if (i->flavor == CapabilityFlavor::Abstract)
+                continue;
+
+            // Try to pack `_atom` and `atom` into the same per key List
+            String name = i->name;
+            if(i->name.startsWith("_"))
+                name = name.subString(1, name.getLength()-1);
+            nameToInternalAndExternalAtom[name].add(i);
+        }
+        for(auto i : nameToInternalAndExternalAtom)
+        {
+            SLANG_ASSERT(i.second.getCount() <= 2);
+            if(i.second.getCount() != 2)
+            {
+                // If we only have a '_Internal' atom inside our name list there is a missing 'External' atom
+                if(i.second[0]->name.startsWith("_"))
+                    m_sink->diagnose(i.second[0]->sourceLoc, Diagnostics::missingExternalInternalAtomPair, i.second[0]->name);
+            }
+        }
+    }
     SlangResult parseDefs()
     {
         auto tokens = m_lexer->lexAllSemanticTokens();
@@ -337,6 +370,7 @@ struct CapabilityDefParser
 
             def->sourceLoc = nameToken.loc;
         }
+        validateInternalAtomExternalAtomPair();
         return SLANG_OK;
     }
 };
@@ -856,12 +890,12 @@ SlangResult generateDefinitions(DiagnosticSink* sink, List<RefPtr<CapabilityDef>
     {
         if (!def)
         {
-            sbCpp << R"(    { "Invalid", CapabilityNameFlavor::Concrete, CapabilityName::Invalid, 0, {nullptr, 0} },)" << "\n";
+            sbCpp << R"(    { UnownedStringSlice::fromLiteral("Invalid"), CapabilityNameFlavor::Concrete, CapabilityName::Invalid, 0, {nullptr, 0} },)" << "\n";
             continue;
         }
 
         // name.
-        sbCpp << "    { \"" << def->name << "\", ";
+        sbCpp << "    { UnownedStringSlice::fromLiteral(\"" << def->name << "\"), ";
 
         // flavor.
         switch (def->flavor)
diff --git a/tools/slang-capability-generator/slang-capability-diagnostic-defs.h b/tools/slang-capability-generator/slang-capability-diagnostic-defs.h
index 1415b1f59..b6c4425a2 100644
--- a/tools/slang-capability-generator/slang-capability-diagnostic-defs.h
+++ b/tools/slang-capability-generator/slang-capability-diagnostic-defs.h
@@ -56,4 +56,5 @@ DIAGNOSTIC(20003, Error, undefinedIdentifier, "undefined identifier \"$0\".")
 DIAGNOSTIC(20004, Error, redefinition, "capability redefinition: '$0'.")
 DIAGNOSTIC(20005, Error, unionWithSameKeyAtomButNotSubset, "unioning ('|') capability sets which have incompatible atoms but compatible 'key atoms', this: '$0', other: '$1'")
 DIAGNOSTIC(20006, Error, invalidJoinInGenerator, "joining ('+') capability sets which have incompatible 'key atoms'")
+DIAGNOSTIC(20007, Error, missingExternalInternalAtomPair, "All internal '_atom' require a corresponding external 'atom' atom meant for user's use. Offending atom: $0")
 #undef DIAGNOSTIC