Add an attribute to disable the overlapping-bindings warning (shader-slang#1005)

Currently if the user gives two global shader parameters conflicting bindings, they get a warning diagnostic:

```hlsl
Texture2D a : register(t0);
Texture2D b : register(t0); // WARNING: overlapping bindings
```

This change adds a way to locally disable that warning using an attribute:

```hlsl
[allow("overlapping-bindings")] Texture2D a : register(t0);
[allow("overlapping-bindings")] Texture2D b : register(t0); // OK
```

Note that as a policy decision, the implementation requires `[allow("overlapping-bindings")]` on both declarations in order to disable the warning, under the assumption that the behavior should be strictly opt-in, and not silently affect a programmer who adds a new shader parameter with no knowledge or expectation of possible overlap.

The `[allow(...)]` attribute is intended to be a fairly generally mechanism for disabling optional diagnostics within certain scopes (e.g., for the body of a function definition), but as implemented in this change it is quite restrictive:

* Only the single name `"overlapping-bindings"` will be recognized, and this name cannot be used with, e.g., a `-W` flag on the command line to enable/disable the same diagnostic, or turn it into an error. Adding more cases would be easy enough, but wiring it up to command-line flags could be trickier.

* Only the code that checks for parameter binding overlap is currently checking for `[allow(...)]` attributes, so it is not "wired up" to enable/disable any others. Doing this systematically would ideally involve something in `diagnose()`, but there could be complications to a systematic approach (finding the AST node(s) to use when searching for `[allow(...)]`.

On gotcha here is that versions of Slang without this feature will error out on the `[allow(...)]` attribute since they don't understand it, and if we add future diagnostics that it covers then old compiler versions will (as written) error out on a diagnostic they haven't heard of rather than just assume the `[allow(...)]` attribute doesn't apply to them. These kinds of issues can and should be addressed in future changes.

Author: Tim Foley

source/slang/core.meta.slang

@@ -1314,3 +1314,7 @@ attribute_syntax [__AttributeUsage(target : _AttributeTargets)] : AttributeUsage
 
 __attributeTarget(VarDeclBase)
 attribute_syntax [format(format : String)] : FormatAttribute;
+
+__attributeTarget(Decl)
+attribute_syntax [allow(diagnostic: String)] : AllowAttribute;
+

source/slang/core.meta.slang.h

@@ -1345,3 +1345,7 @@ SLANG_RAW("attribute_syntax [__AttributeUsage(target : _AttributeTargets)] : Att
 SLANG_RAW("\n")
 SLANG_RAW("__attributeTarget(VarDeclBase)\n")
 SLANG_RAW("attribute_syntax [format(format : String)] : FormatAttribute;\n")
+SLANG_RAW("\n")
+SLANG_RAW("__attributeTarget(Decl)\n")
+SLANG_RAW("attribute_syntax [allow(diagnostic: String)] : AllowAttribute;\n")
+SLANG_RAW("\n")

source/slang/slang-check.cpp

@@ -2934,6 +2934,24 @@ namespace Slang
 
                     formatAttr->format = format;
                 }
+                else if (auto allowAttr = as<AllowAttribute>(attr))
+                {
+                    SLANG_ASSERT(attr->args.getCount() == 1);
+
+                    String diagnosticName;
+                    if(!checkLiteralStringVal(attr->args[0], &diagnosticName))
+                    {
+                        return false;
+                    }
+
+                    auto diagnosticInfo = findDiagnosticByName(diagnosticName.getUnownedSlice());
+                    if(!diagnosticInfo)
+                    {
+                        getSink()->diagnose(attr->args[0], Diagnostics::unknownDiagnosticName, diagnosticName);
+                    }
+
+                    allowAttr->diagnostic = diagnosticInfo;
+                }
                 else
                 {
                     if(attr->args.getCount() == 0)

source/slang/slang-diagnostic-defs.h

@@ -269,6 +269,7 @@ DIAGNOSTIC(31006, Error, attributeFunctionNotFound, "Could not find function '$0
 
 DIAGNOSTIC(31100, Error, unknownStageName, "unknown stage name '$0'")
 DIAGNOSTIC(31101, Error, unknownImageFormatName, "unknown image format '$0'")
+DIAGNOSTIC(31101, Error, unknownDiagnosticName, "unknown diagnostic '$0'")
 
 DIAGNOSTIC(31120, Error, invalidAttributeTarget, "invalid syntax target for user defined attribute")
 

source/slang/slang-diagnostics.cpp

@@ -339,12 +339,37 @@ void DiagnosticSink::diagnoseRaw(
     }
 }
 
-
 namespace Diagnostics
 {
 #define DIAGNOSTIC(id, severity, name, messageFormat) const DiagnosticInfo name = { id, Severity::severity, messageFormat };
 #include "slang-diagnostic-defs.h"
 }
 
+DiagnosticInfo const* findDiagnosticByName(UnownedStringSlice const& name)
+{
+    // TODO: We should eventually have a more formal system for associating individual
+    // diagnostics, or groups of diagnostics, with user-exposed names for use when
+    // enabling/disabling warnings (or turning warnings into errors, etc.).
+    //
+    // For now we build an ad hoc mapping from string names to corresponding single
+    // diagnostics (not groups).
+    //
+    static const struct
+    {
+        char const*             name;
+        DiagnosticInfo const*   diagnostic;
+    } kDiagnostics[] =
+    {
+        { "overlapping-bindings", &Diagnostics::parameterBindingsOverlap },
+    };
+
+    for( auto& entry : kDiagnostics )
+    {
+        if( name == entry.name )
+            return entry.diagnostic;
+    }
+    return nullptr;
+}
+
 
 } // namespace Slang

source/slang/slang-diagnostics.h

@@ -257,6 +257,8 @@ namespace Slang
         DiagnosticSink* m_sink = nullptr;
     };
 
+    DiagnosticInfo const* findDiagnosticByName(UnownedStringSlice const& name);
+
     namespace Diagnostics
     {
 #define DIAGNOSTIC(id, severity, name, messageFormat) extern const DiagnosticInfo name;

source/slang/slang-modifier-defs.h

@@ -461,3 +461,7 @@ END_SYNTAX_CLASS()
 SYNTAX_CLASS(FormatAttribute, Attribute)
     FIELD(ImageFormat, format)
 END_SYNTAX_CLASS()
+
+SYNTAX_CLASS(AllowAttribute, Attribute)
+    FIELD_INIT(DiagnosticInfo const*, diagnostic, nullptr)
+END_SYNTAX_CLASS()

source/slang/slang-parameter-binding.cpp

@@ -784,6 +784,25 @@ static UInt allocateUnusedSpaces(
     return context->shared->usedSpaces.Allocate(nullptr, count);
 }
 
+static bool shouldDisableDiagnostic(
+    Decl*                   decl,
+    DiagnosticInfo const&   diagnosticInfo)
+{
+    for( auto dd = decl; dd; dd = dd->ParentDecl )
+    {
+        for( auto modifier : dd->modifiers )
+        {
+            auto allowAttr = as<AllowAttribute>(modifier);
+            if(!allowAttr)
+                continue;
+
+            if(allowAttr->diagnostic == &diagnosticInfo)
+                return true;
+        }
+    }
+    return false;
+}
+
 static void addExplicitParameterBinding(
     ParameterBindingContext*    context,
     RefPtr<ParameterInfo>       parameterInfo,
@@ -842,11 +861,26 @@ static void addExplicitParameterBinding(
             auto paramA = parameterInfo->varLayouts[0]->varDecl.getDecl();
             auto paramB = overlappedVarLayout->varDecl.getDecl();
 
-            getSink(context)->diagnose(paramA, Diagnostics::parameterBindingsOverlap,
-                getReflectionName(paramA),
-                getReflectionName(paramB));
+            auto& diagnosticInfo = Diagnostics::parameterBindingsOverlap;
+
+            // If *both* of the shader parameters declarations agree
+            // that overlapping bindings should be allowed, then we
+            // will not emit a diagnostic. Otherwise, we will warn
+            // the user because such overlapping bindings are likely
+            // to indicate a programming error.
+            //
+            if(shouldDisableDiagnostic(paramA, diagnosticInfo)
+                && shouldDisableDiagnostic(paramB, diagnosticInfo))
+            {
+            }
+            else
+            {
+                getSink(context)->diagnose(paramA, diagnosticInfo,
+                    getReflectionName(paramA),
+                    getReflectionName(paramB));
 
-            getSink(context)->diagnose(paramB, Diagnostics::seeDeclarationOf, getReflectionName(paramB));
+                getSink(context)->diagnose(paramB, Diagnostics::seeDeclarationOf, getReflectionName(paramB));
+            }
         }
     }
 }

tests/diagnostics/overlapping-bindings.slang

@@ -0,0 +1,17 @@
+// overlapping-bindings.slang
+
+//DIAGNOSTIC_TEST:SIMPLE:-target hlsl
+
+// Two parameters with the same `register:
+
+Texture2D a : register(t0);
+
+Texture2D b : register(t0);
+
+// Parameters marked to ignore overlap:
+
+[allow("overlapping-bindings")]
+Texture2D c : register(t1);
+
+[allow("overlapping-bindings")]
+Texture2D d : register(t1);

tests/diagnostics/overlapping-bindings.slang.expected

@@ -0,0 +1,7 @@
+result code = 0
+standard error = {
+tests/diagnostics/overlapping-bindings.slang(9): warning 39001: explicit binding for parameter 'b' overlaps with parameter 'a'
+tests/diagnostics/overlapping-bindings.slang(7): note: see declaration of 'a'
+}
+standard output = {
+}