Fix spurious vk::binding warnings when attribute is present (#7581)

* Initial plan

* Fix spurious vk::binding warnings when attribute is present

- Modified _maybeDiagnoseMissingVulkanLayoutModifier to check for GLSLBindingAttribute before warning
- Changed function to return bool indicating if warning was actually issued
- Updated call sites to properly track warning state to reduce duplicates
- Tested fix resolves the issue while preserving correct warnings when needed

Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>

* Add test case for vk::binding spurious warning fix

Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>

* Update test to use filecheck format as requested

Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>

* Remove full file path from CHECK directives as requested

Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>

* Fix code formatting with clang-format

Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>

* Fix behavioral regression in vk-bindings test caused by warning flag logic

Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>
Co-authored-by: Yong He <yonghe@outlook.com>

1 files changed, 24 insertions, 9 deletions

diff --git a/source/slang/slang-parameter-binding.cpp b/source/slang/slang-parameter-binding.cpp
index 86d76fc6d..4ba9e4e03 100644
--- a/source/slang/slang-parameter-binding.cpp
+++ b/source/slang/slang-parameter-binding.cpp
@@ -1069,7 +1069,7 @@ static void addExplicitParameterBindings_HLSL(
     }
 }
 
-static void _maybeDiagnoseMissingVulkanLayoutModifier(
+static bool _maybeDiagnoseMissingVulkanLayoutModifier(
     ParameterBindingContext* context,
     DeclRef<VarDeclBase> const& varDecl)
 {
@@ -1077,7 +1077,7 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier(
     if (varDecl.getDecl()->hasModifier<PushConstantAttribute>() ||
         varDecl.getDecl()->hasModifier<ShaderRecordAttribute>())
     {
-        return;
+        return false;
     }
 
     // If the user didn't specify a `binding` (and optional `set`) for Vulkan,
@@ -1085,6 +1085,11 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier(
     // oversight on their part.
     if (auto registerModifier = varDecl.getDecl()->findModifier<HLSLRegisterSemantic>())
     {
+        // Don't warn if the declaration already has a vk::binding attribute
+        if (varDecl.getDecl()->findModifier<GLSLBindingAttribute>())
+        {
+            return false;
+        }
         auto varType = getType(context->getASTBuilder(), varDecl.as<VarDeclBase>());
         if (auto textureType = as<TextureType>(varType))
         {
@@ -1095,7 +1100,7 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier(
                     registerModifier,
                     Diagnostics::registerModifierButNoVulkanLayout,
                     varDecl.getName());
-                return;
+                return true;
             }
         }
 
@@ -1111,7 +1116,9 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier(
             Diagnostics::registerModifierButNoVkBindingNorShift,
             varDecl.getName(),
             registerClassName);
+        return true;
     }
+    return false;
 }
 
 static void addExplicitParameterBindings_GLSL(
@@ -1273,8 +1280,16 @@ static void addExplicitParameterBindings_GLSL(
     // If we are not told how to infer bindings with a compile option, we warn
     if (hlslToVulkanLayoutOptions == nullptr || !hlslToVulkanLayoutOptions->canInferBindings())
     {
-        warnedMissingVulkanLayoutModifier = true;
-        _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
+        warnedMissingVulkanLayoutModifier =
+            _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
+        // For behavioral compatibility, if we didn't warn due to a push constant or shader record,
+        // we still need to set the flag to true to maintain the same binding logic
+        if (!warnedMissingVulkanLayoutModifier &&
+            (varDecl.getDecl()->hasModifier<PushConstantAttribute>() ||
+             varDecl.getDecl()->hasModifier<ShaderRecordAttribute>()))
+        {
+            warnedMissingVulkanLayoutModifier = true;
+        }
     }
 
     // We need an HLSL register semantic to to infer from
@@ -1301,8 +1316,8 @@ static void addExplicitParameterBindings_GLSL(
         {
             if (!warnedMissingVulkanLayoutModifier)
             {
-                _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
-                warnedMissingVulkanLayoutModifier = true;
+                warnedMissingVulkanLayoutModifier =
+                    _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
             }
             return;
         }
@@ -1323,8 +1338,8 @@ static void addExplicitParameterBindings_GLSL(
     {
         if (!warnedMissingVulkanLayoutModifier)
         {
-            _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
-            warnedMissingVulkanLayoutModifier = true;
+            warnedMissingVulkanLayoutModifier =
+                _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
         }
     }