Add warning for ignored binding attributes on uniforms (#6373)

Fixes #4251

When binding attributes (like [[vk::binding]]) are specified on uniforms
that get packed into the default constant buffer, these binding
attributes are effectively ignored since the uniform will always be
placed at descriptor set 0, binding 0. This can be confusing for users
who expect their explicit bindings to take effect.

This change adds a new warning (71) that informs users when their
binding attributes on uniforms will be ignored, and suggests declaring
the uniform inside a constant buffer to preserve the explicit binding.

The warning helps users understand:
1. Why their binding attribute isn't having the expected effect
2. That the uniform is being packed into the default constant buffer
3. How to fix it by using a constant buffer declaration

Added test case in tests/bugs/binding-attribute-ignored.slang to verify
the warning behavior.

Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>

Author: mkeshavaNV

source/slang/slang-diagnostic-defs.h

@@ -2055,6 +2055,14 @@ DIAGNOSTIC(
     "shader parameter '$0' has a 'register' specified for D3D, but no '[[vk::binding(...)]]` "
     "specified for Vulkan, nor is `-fvk-$1-shift` used.")
 
+DIAGNOSTIC(
+    39071,
+    Warning,
+    bindingAttributeIgnoredOnUniform,
+    "binding attribute on uniform '$0' will be ignored since it will be packed into the default "
+    "constant buffer at descriptor set 0 binding 0. To use explicit bindings, declare the uniform "
+    "inside a constant buffer.")
+
 //
 
 // 4xxxx - IL code generation.

source/slang/slang-parameter-binding.cpp

@@ -3511,34 +3511,22 @@ static void collectParameters(ParameterBindingContext* inContext, ComponentType*
 /// Emit a diagnostic about a uniform/ordinary parameter at global scope.
 void diagnoseGlobalUniform(SharedParameterBindingContext* sharedContext, VarDeclBase* varDecl)
 {
-    // This subroutine gets invoked if a shader parameter containing
-    // "ordinary" data (sometimes just called "uniform" data) is present
-    // at the global scope.
-    //
-    // Slang can support such parameters by aggregating them into
-    // an implicit constant buffer, but it is also common for programmers
-    // to accidentally declare a global-scope shader parameter when they
-    // meant to declare a global variable instead:
-    //
-    //      int gCounter = 0; // this is a shader parameter, not a global
-    //
-    // In order to avoid mistakes, we'd like to warn the user when
-    // they write code like the above, and hint to them that they
-    // should make their intention more explicit with a keyword:
-    //
-    //      static int gCounter = 0; // this is now a (static) global
-    //
-    //      uniform int gCounter; // this is now explicitly a shader parameter
-    //
-    // We skip the diagnostic whenever the variable was explicitly `uniform`,
-    // under the assumption that the programmer who added that modifier
-    // knew what they were opting into.
-    //
-    if (varDecl->hasModifier<HLSLUniformModifier>())
-        return;
+    // Don't emit the implicit global shader parameter warning if the variable is explicitly marked
+    // as uniform
+    if (!varDecl->hasModifier<HLSLUniformModifier>())
+    {
+        getSink(sharedContext)
+            ->diagnose(varDecl, Diagnostics::globalUniformNotExpected, varDecl->getName());
+    }
 
-    getSink(sharedContext)
-        ->diagnose(varDecl, Diagnostics::globalUniformNotExpected, varDecl->getName());
+    // Always check and warn about binding attributes being ignored, regardless of uniform modifier
+    if (varDecl->findModifier<GLSLBindingAttribute>())
+    {
+        sharedContext->m_sink->diagnose(
+            varDecl,
+            Diagnostics::bindingAttributeIgnoredOnUniform,
+            varDecl->getName());
+    }
 }
 
 

tests/bugs/binding-attribute-ignored.slang

@@ -0,0 +1,22 @@
+// binding-attribute-ignored.slang
+// Test that binding attributes on uniforms that get packed into the default uniform buffer trigger a warning
+
+//TEST:SIMPLE(filecheck=CHECK):-target spirv
+
+//CHECK: ([[# @LINE+2]]): warning 39071
+[[vk::binding(1, 2)]]
+uniform float4 g_position;
+
+//CHECK: ([[# @LINE+2]]): warning 39071
+[[vk::binding(3, 1)]]
+uniform float4x4 g_transform;
+
+// This won't trigger a warning because it's a texture (not packed into default uniform buffer)
+[[vk::binding(0, 0)]]
+Texture2D g_texture;
+
+[shader("vertex")]
+float4 main(float4 pos : POSITION) : SV_POSITION
+{
+    return g_position;
+}