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>

1 files changed, 15 insertions, 27 deletions

diff --git a/source/slang/slang-parameter-binding.cpp b/source/slang/slang-parameter-binding.cpp
index 67ce46e8f..0e47ff56d 100644
--- a/source/slang/slang-parameter-binding.cpp
+++ b/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());
+    }
 }