diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2019-07-29 13:21:32 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2019-07-29 13:21:32 -0700 |
| commit | 9f33d3d4eb81afde1b4445a4251979eeb5436e7f (patch) | |
| tree | 6eb52affab80294535a9f9837bbfb8d9ad4db4b5 /tests | |
| parent | ade2c39fa3675504ed135ef8abe7b53cfd06ee84 (diff) | |
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.
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/diagnostics/overlapping-bindings.slang | 17 | ||||
| -rw-r--r-- | tests/diagnostics/overlapping-bindings.slang.expected | 7 |
2 files changed, 24 insertions, 0 deletions
diff --git a/tests/diagnostics/overlapping-bindings.slang b/tests/diagnostics/overlapping-bindings.slang new file mode 100644 index 000000000..8df588d16 --- /dev/null +++ b/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); diff --git a/tests/diagnostics/overlapping-bindings.slang.expected b/tests/diagnostics/overlapping-bindings.slang.expected new file mode 100644 index 000000000..80481eaf9 --- /dev/null +++ b/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 = { +} |
