More precise grammar for conditional translation

[stefnotch]

Conditional translation in the spec and in the implementations does not fully match up. Time to fix this.

Narrower grammar for @if

#191 (comment)

Both in wesl-rs and wgsl-analyzer, the current grammar of @if has been causing implementation woes. To address this with the smallest spec change, I opted to separate "attributes" and "translate-time attributes". Translate-time attributes are @if(...), @elif(...) and @else. They are parsed exactly as such, thus being unambiguous. This change does not reject any existing WESL programs.

@else in the spec

#117

Both wesl-js and wesl-rs support @elif and @else. This adds them to the spec

Compared to #164 , I did not yet update the name to "condition attributes". I wanted to keep the diff easy to review for this PR.

[k2d222]

clarify what unambiguously specified means. I think you mean: t-t attributes are a distinct prodution rule from regular attributes, because regular attrs would be ambiguous in certain locations. Concrete example...

[k2d222]

you can also make this a note with [!NOTE]

[stefnotch]

I'm not sure how to phrase this in a succinct way. #191 is kind of long.

[k2d222]

I mean it's not at all obvious that the grammar is unambiguous here, and I genuinely wasn't sure if/why it was at first. The root of the ambiguity is the optional ( in the attribute production rule, which is ambiguous with statements starting with (. It would not be ambiguous if ( was mandatory or forbidden.

But your solution introduces a context-sensitive token: 'elif'. Before this change, one could just parse all attributes as '@' Ident, now the lexer has to track if the previous token is @. No big deal, but we should mention it.

But if we use context-sensitive lexing, then why not make all attribute names context-sensitive, remove the catch-all rule (@ Ident OptionalParen), and then the ( is no longer optional, and the ambiguity disappears! We don't need the separate translate_time_attribute rule, Hooray! ...But that only works because each attribute has either no paren or a mandatory paren, so it's a bit fragile if we do introduce one in the future.

But actually it can work without context-sensitive lexing, if you rewrite translate_time_attribute as '@' 'if' Paren | '@' 'else' | '@' Ident Paren, making the ( non-optional. But then, the attribute must become just translate_time_attribute | '@' Ident, because the '@' Ident Paren is already taken. And the name translate_time_attribute is no longer accurate.

TL;DR: idk what the best solution is here, but these are the options:

• keep syntax as-is, but tell that elif is context-sensitive, and that translate-time-attribute is a sub-rule of attribute.
• make all attributes context-sensitive, and have just one attribute rule everywhere.
• make attributes non-context-sensitive, and rewrite/rename the two-stage ambiguous/unambiguous attribute rules.

[stefnotch]

Ah, I had missed that. I chose your final option without context sensitivity. I did it with one extra rule, hopefully that's reasonably clear.

(Although I'm not absolutely sure if that really solves it.)

[k2d222]

I should have added 1 more detail: the WGSL grammar already says that attribute names are context-sensitive. But it implicitly leaves room for implementations to be non-context-sensitive, by adding a catch-all rule for "unknown" attributes.

Your commit does not solve it, here's how to solve it:

• You want elif to not be context-sensitive, so you must have '@' ident_pattern_token argument_expression_list in the 'translate_time_attribute' rule.
• You want attribute to contain translate_time_attribute, but they must not conflict (you can't allow a syntax form in attribute that's already in translate_time_attribute), so you must replace '@' ident_pattern_token argument_expression_list ? with just '@' ident_pattern_token

solution:

attribute:
    // the rule `'@' ident_pattern_token argument_expression_list` is already in `unambiguous_attribute`, having it here too would be a Reduce-Reduce conflict.
    | unambiguous_attribute
    | '@' ident_pattern_token
    | ...

unambiguous_attribute:
    | '@' ident_pattern_token argument_expression_list
    // the WGSL grammar for attributes contains both a catch-all production rule, and more specific rules *if you support* context-sensitive tokens. If your lexer *does not support* them, you can just skip the rules below.
    | '@' 'if' '(' expression ',' ? ')'
    | '@' 'elif' '(' expression ',' ? ')'
    | '@' 'else'

[stefnotch]

Wait, I don't get how that is different?

unambiguous_attribute corresponds to translate_time_attribute (if you ignore the indirection)

And attribute also expands to the same set of rules. It has a '@' ident_pattern_token argument_expression_list ?

[k2d222]

for a bottom-up LR parser, you can't have two child rules that accept the same syntax phrase.

In your commit, you have translate_time_attribute which accepts '@' Ident argument_expression_list, and attribute which accepts '@' Ident argument_expression_list ?. The latter has the extra ?, but they both accept "@attr(expr)".

the pedantic explanation: when a parser is in a state where it has read @ ident argument_expression_list, and can reduce it to an attribute, then necessarily it can also reduce it to a translate_time_attribute, because the latter is a sub-rule of attribute, so it must be available at that point. It doesn't know whether to do @ ident argument_expression_list -> attribute, or @ ident argument_expression_list -> translate_time_attribute -> attribute.

The smallest fix from your PR is simply changing '@' Ident argument_expression_list ? to '@' Ident in attribute, but notice that it also changes the semantics of translate_time_attribute, which now also captures all attributes with parenthesis. That's why I renamed it.

[stefnotch]

Oh I understand. If I see a @, I do not yet know whether I'll parse a translate_time_attribute or a normal attribute. So the rules do conflict.

Then I guess I'll apply your fix.

As for the alternative of context sensitive lexing: It adds so much baggage to the lexers that I prefer to avoid it.

[k2d222]

yeah, and the WGSL spec is a tiny bit vague here. They say attributes are context-sensitive, but they do provide this catch-all grammar rule which serve no purpose, whithout explaining that it's an escape hatch for non-context-sensitive lexers.