From 93ac152bf8283eff1d1b2ec0f7df897a4e96464f Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Fri, 16 Mar 2018 07:01:35 -0700 Subject: Overhaul implementation of [attributes] (#443) 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. --- source/slang/parser.cpp | 131 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 130 insertions(+), 1 deletion(-) (limited to 'source/slang/parser.cpp') diff --git a/source/slang/parser.cpp b/source/slang/parser.cpp index 82f79af17..5fb7445e9 100644 --- a/source/slang/parser.cpp +++ b/source/slang/parser.cpp @@ -602,10 +602,20 @@ namespace Slang parser->ReadToken(TokenType::LBracket); for(;;) { + // Note: When parsing we just construct an AST node for an + // "unchecked" attribute, and defer all detailed semantic + // checking until later. + // + // An alternative would be to perform lookup of an `AttributeDecl` + // at this point, similar to what we do for `SyntaxDecl`, but it + // seems better to not complicate the parsing process any more. + // + auto nameToken = parser->ReadToken(TokenType::Identifier); - RefPtr modifier = new HLSLUncheckedAttribute(); + RefPtr modifier = new UncheckedAttribute(); modifier->name = nameToken.getName(); modifier->loc = nameToken.getLoc(); + modifier->scope = parser->currentScope; if (AdvanceIf(parser, TokenType::LParent)) { @@ -2500,6 +2510,107 @@ namespace Slang return syntaxDecl; } + // A parameter declaration in an attribute declaration. + // + // We are going to use `name: type` syntax just for simplicty, and let the type + // be optional, because we don't actually need it in all cases. + // + static RefPtr parseAttributeParamDecl(Parser* parser) + { + auto nameAndLoc = expectIdentifier(parser); + + RefPtr paramDecl = new ParamDecl(); + paramDecl->nameAndLoc = nameAndLoc; + + if(AdvanceIf(parser, TokenType::Colon)) + { + paramDecl->type = parser->ParseTypeExp(); + } + + if(AdvanceIf(parser, TokenType::OpAssign)) + { + paramDecl->initExpr = parser->ParseInitExpr(); + } + + return paramDecl; + } + + // Parse declaration of a name to be used for resolving `[attribute(...)]` style modifiers. + // + // These are distinct from `syntax` declarations, because their names don't get added + // to the current scope using their default name. + // + // Also, attribute-specific code doesn't get invokved during parsing. We always parse + // using the default attribute-parsing logic and then all specialized behavior takes + // place during semantic checking. + // + static RefPtr parseAttributeSyntaxDecl(Parser* parser, void* /*userData*/) + { + // Right now the basic form is: + // + // attribute_syntax : ; + // + // - `name` gives the name of the attribute to define. + // - `syntaxClass` is the name of an AST node class that we expect + // this attribute to create when checked. + // - `existingKeyword` is the name of an existing keyword that + // the new syntax should be an alias for. + + expect(parser, TokenType::LBracket); + + // First we parse the attribute name. + auto nameAndLoc = expectIdentifier(parser); + + RefPtr attrDecl = new AttributeDecl(); + if(AdvanceIf(parser, TokenType::LParent)) + { + while(!AdvanceIfMatch(parser, TokenType::RParent)) + { + auto param = parseAttributeParamDecl(parser); + + AddMember(attrDecl, param); + + if(AdvanceIfMatch(parser, TokenType::RParent)) + break; + + expect(parser, TokenType::Comma); + } + } + + expect(parser, TokenType::RBracket); + + // TODO: we should allow parameters to be specified here, to cut down + // on the amount of per-attribute-type logic that has to occur later. + + // Next we look for a clause that specified the AST node class. + SyntaxClass syntaxClass; + if (AdvanceIf(parser, TokenType::Colon)) + { + // User is specifying the class that should be construted + auto classNameAndLoc = expectIdentifier(parser); + + syntaxClass = parser->getSession()->findSyntaxClass(classNameAndLoc.name); + } + else + { + // For now we don't support the alternative approach where + // an existing piece of syntax is named to provide the parsing + // support. + + // TODO: diagnose: a syntax class must be specified. + } + + expect(parser, TokenType::Semicolon); + + // TODO: skip creating the declaration if anything failed, just to not screw things + // up for downstream code? + + attrDecl->nameAndLoc = nameAndLoc; + attrDecl->loc = nameAndLoc.loc; + attrDecl->syntaxClass = syntaxClass; + return attrDecl; + } + // Finish up work on a declaration that was parsed static void CompleteDecl( Parser* /*parser*/, @@ -4182,6 +4293,20 @@ namespace Slang return modifier; } + static RefPtr parseAttributeTargetModifier(Parser* parser, void* /*userData*/) + { + expect(parser, TokenType::LParent); + auto syntaxClassNameAndLoc = expectIdentifier(parser); + expect(parser, TokenType::RParent); + + auto syntaxClass = parser->getSession()->findSyntaxClass(syntaxClassNameAndLoc.name); + + RefPtr modifier = new AttributeTargetModifier(); + modifier->syntaxClass = syntaxClass; + + return modifier; + } + RefPtr populateBaseLanguageModule( Session* session, RefPtr scope) @@ -4205,6 +4330,7 @@ namespace Slang DECL(__subscript, ParseSubscriptDecl); DECL(interface, parseInterfaceDecl); DECL(syntax, parseSyntaxDecl); + DECL(attribute_syntax,parseAttributeSyntaxDecl); DECL(__import, parseImportDecl); DECL(import, parseImportDecl); @@ -4280,6 +4406,9 @@ namespace Slang MODIFIER(__intrinsic_type, parseIntrinsicTypeModifier); MODIFIER(__implicit_conversion, parseImplicitConversionModifier); + MODIFIER(__attributeTarget, parseAttributeTargetModifier); + + #undef MODIFIER // Add syntax for expression keywords -- cgit v1.2.3