From 9f33d3d4eb81afde1b4445a4251979eeb5436e7f Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Mon, 29 Jul 2019 13:21:32 -0700 Subject: Add an attribute to disable the overlapping-bindings warning (#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. --- source/slang/slang-check.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'source/slang/slang-check.cpp') diff --git a/source/slang/slang-check.cpp b/source/slang/slang-check.cpp index a8900986b..534642a1c 100644 --- a/source/slang/slang-check.cpp +++ b/source/slang/slang-check.cpp @@ -2934,6 +2934,24 @@ namespace Slang formatAttr->format = format; } + else if (auto allowAttr = as(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) -- cgit v1.2.3