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>

3 files changed, 61 insertions, 11 deletions

diff --git a/source/slang-glslang/slang-glslang.cpp b/source/slang-glslang/slang-glslang.cpp
index e830fb15a..56be8b042 100644
--- a/source/slang-glslang/slang-glslang.cpp
+++ b/source/slang-glslang/slang-glslang.cpp
@@ -189,7 +189,7 @@ extern "C"
 #ifdef _MSC_VER
     _declspec(dllexport)
 #else
-__attribute__((__visibility__("default")))
+    __attribute__((__visibility__("default")))
 #endif
         bool glslang_disassembleSPIRVWithResult(
             const uint32_t* contents,
@@ -237,7 +237,7 @@ extern "C"
 #ifdef _MSC_VER
     _declspec(dllexport)
 #else
-__attribute__((__visibility__("default")))
+    __attribute__((__visibility__("default")))
 #endif
         bool glslang_disassembleSPIRV(const uint32_t* contents, int contentsSize)
 {
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>());
         }
     }
 
diff --git a/tests/diagnostics/vk-binding-with-register-no-warning.slang b/tests/diagnostics/vk-binding-with-register-no-warning.slang
new file mode 100644
index 000000000..3bbbcc1bd
--- /dev/null
+++ b/tests/diagnostics/vk-binding-with-register-no-warning.slang
@@ -0,0 +1,35 @@
+// vk-binding-with-register-no-warning.slang
+
+//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK):-target metallib -matrix-layout-row-major -entry main
+
+// Test that no spurious warnings are generated when vk::binding is present alongside register
+// This addresses issue #7553 where the compiler incorrectly warned about missing vk::binding
+
+// Should NOT generate warning - has both vk::binding and register
+// CHECK-NOT: ([[# @LINE+1]]): warning 39029
+[[vk::binding(0, 0)]] cbuffer ConstantBuffer : register(b0)
+{
+    float4 testValue;
+};
+
+// Should NOT generate warning - has both vk::binding and register  
+// CHECK-NOT: ([[# @LINE+1]]): warning 39029
+[[vk::binding(1, 0)]] Texture2D tex : register(t1);
+
+// Should NOT generate warning - has both vk::binding and register
+// CHECK-NOT: ([[# @LINE+1]]): warning 39029
+[[vk::binding(2, 0)]] SamplerState sampler : register(s2);
+
+// Should generate warning - has register but no vk::binding
+// CHECK: ([[# @LINE+1]]): warning 39029
+Texture2D texNoBinding : register(t3);
+
+// Should generate warning - has register but no vk::binding
+// CHECK: ([[# @LINE+1]]): warning 39029
+ConstantBuffer<float4> cbNoBinding : register(b4);
+
+[shader("compute")]
+[numthreads(1, 1, 1)]
+void main()
+{
+}
\ No newline at end of file