https://github.com/shader-slang/slang
Revision 93ac152bf8283eff1d1b2ec0f7df897a4e96464f authored by Tim Foley on 16 March 2018, 14:01:35 UTC, committed by GitHub on 16 March 2018, 14:01:35 UTC
The existing code parsed all of the square-bracket `[attributes]` into `HLSLUncheckedAttribute`, and then went on to hand-convert some of them to specialized subclasses of `HLSLAttribute`. When attributes didn't check, they were left as-is, and no error message was issued, because at the time the compiler was focused on accepting arbitrary input.

This change greatly overhauls the handling of `[attributes]`. Attributes are now declared in the stdlib, with declarations like:

```hlsl
__attributeTarget(LoopStmt)
attribute_syntax [unroll(count: int = 0)] : UnrollAttribute;
```

In this syntax, the `unroll` part is giving the attribute name (the `[]` are just for flavor, to make the declaration look like a use site; we could drop it if we don't like the clutter), the `count` is a parameter of the attribute, which we expect to be of type `int`, and which has a default value of `0` if unspecified.

The `: UnrollAttribute` part specifies the meta-level C++ class that will implement this attribute (and corresponds to a class in `modifier-defs.h`). This syntax is similar to our current `syntax` declarations. I'm starting to think we should change it to something like a `__meta_class(UnrollAttribute)` modifier, and then use that uniformly across all cases (e.g., also replacing the curreent `__magic_type(Foo)` syntax).

The `__attributeTarget(LoopStmt)` is a modifier that specifies the meta-level C++ class for syntax that this attribute is allowed to attach to. It is legal to have more than one of these.

Attributes continue to be parsed in an unchecked form, so that we don't tie up semantic analysis and parsing more than necessary. During checking, we look up the attribute name in the current scope, and then replace the unchecked attribute with a more specific one *if* the checking passes.

Checking proceeds in generic and attribute-specific phases. The generic phase includes checking the number of arguments against those specified in the attribute declaration (I don't currently check types, or handle default arguments), and then checking that at least one `__attributeTarget(...)` modifier applies to the syntax node being modified.

The attribute-specific phase then applies to the specialized C++ subclass of `Attribute`, and does the actual checking right now (e.g., that step is responsible for actually type-checking things at present). This can obviously be improved over time.

With this support I went ahead and added declarations for all the HLSL attributes I could find documented on MSDN. I also added a provisional declaration for the `[shader(...)]` attribute that has been added to dxc, but which is not yet documented.

One important detail here is that lookup of attribute names needs to be done carefully, so that we don't let, e.g., local variables shadow an attribute declaration:

```hlsl
int unroll = 5;

// This attribute should *not* get confused by the local variable `unroll`
[unroll] for(...) { .. }
```

The lookup logic already has a notion of a `LookupMask` that can be used to filter declarations out of the result. In this change I surfaced that mask through the main lookup API (rather than requiring a second pass to "refine" lookup results), and made is so that the default lookup mask does *not* include attributes, while an explicit mask can be used to look up *only* attributes.

(An alternatie design we discussed was to follow the approach of C# and have the declaration of an attribute like `[unroll]` actually be `unrollAttribute`, with a suffix. I decided not to follow that approach for now because it seemed like printing good error messages in that case could require us to carefully trim the `Attribute` suffix off of names at times, and using the existing mask behavior seemed simpler.)

To verify that the shadowing behavior is indeed correct, I modified the `loop-unroll.slang` test case.

Smaller notes:

* Removed the `HLSL` prefix from several of the C++ attribute classes

* Made sure to actually validate the modifiers on statements

* Special-cased checking for `ParamDecl` with a null type, because I'm re-using `ParamDecl` for attribute parameters, but can't give a concrete type to some of them right now

* Deleting some old, dead emit-from-AST logic around attributes, rather than try to "fix" code that doesn't run (a more complete scrub of that code is still needed)

* Fixed AST inheritance hierarchy so that a `Modifier` is a `SyntaxNode` rather than a `SyntaxNodeBase`. I have *no* idea why we have both of those, and we need to clean that up soon.
1 parent e69c0bd
History
Tip revision: 93ac152bf8283eff1d1b2ec0f7df897a4e96464f authored by Tim Foley on 16 March 2018, 14:01:35 UTC
Overhaul implementation of [attributes] (#443)
Tip revision: 93ac152
File Mode Size
build
docs
examples
external
source
tests
tools
.gitattributes -rw-r--r-- 95 bytes
.gitignore -rw-r--r-- 398 bytes
.gitmodules -rw-r--r-- 107 bytes
.travis.yml -rw-r--r-- 1.6 KB
CODE_OF_CONDUCT.md -rw-r--r-- 3.1 KB
LICENSE -rw-r--r-- 1.1 KB
Makefile -rw-r--r-- 6.3 KB
README.md -rw-r--r-- 4.9 KB
appveyor.yml -rw-r--r-- 3.5 KB
slang.h -rw-r--r-- 42.0 KB
slang.sln -rw-r--r-- 9.1 KB
test.bat -rw-r--r-- 1.4 KB

README.md

back to top