Skip to content

Conversation

@RobinMorisset
Copy link
Contributor

The problem is very simple, when seeing:

f(

in a statement context, the parser does not know with bounded lookahead whether it will see a mere function call, or an assignment of the form:

f(1,2,3) = 42;

The latter is obviously not allowed by the language, but currently this restriction is only enforced at the type level instead of the syntax level. I propose adding these restrictions to the syntax to spare the parser from pointless backtracking:

assignment_statement :
    | ( lhs_expression | underscore ) equal short_circuit_or_expression
lhs_expression :
    | ( star | and ) * core_lhs_expression postfix_expression ?
core_lhs_expression :
    | ident
    | paren_left lhs_expression paren_right

Instead of the current approach where practically any expression can be on the left-hand side of an assignment.

I looked at all type rules that allow the creation of memory references (required for the left-hand side of an assignment), and I believe that I have captured all possible such expressions. I tried to verify it by checking that all of our examples still parse with the new grammar.

This allows removing one of the 3 known conflicts in the grammar today which prevent it from being LR(1). I hope to fix the other two in separate PRs.

@RobinMorisset RobinMorisset added the wgsl WebGPU Shading Language Issues label Nov 6, 2021
@RobinMorisset RobinMorisset self-assigned this Nov 6, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2021

Previews, as seen when this build job started (ef0b339):
WebGPU | IDL
WGSL
Explainer

@dneto0 dneto0 self-requested a review November 9, 2021 20:05
Copy link
Contributor

@dneto0 dneto0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but I think we can make it simpler

@dneto0 dneto0 merged commit e445692 into gpuweb:main Nov 9, 2021
github-actions bot added a commit that referenced this pull request Nov 9, 2021
…t-hand-side of assignments. (#2265)

SHA: e445692
Reason: push, by @dneto0

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Nov 9, 2021
…t-hand-side of assignments. (#2265)

SHA: e445692
Reason: push, by @dneto0

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Nov 9, 2021
…t-hand-side of assignments. (#2265)

SHA: e445692
Reason: push, by @dneto0

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@RobinMorisset RobinMorisset deleted the lhsExpression branch November 9, 2021 20:47
@kdashg
Copy link
Contributor

kdashg commented Nov 10, 2021

WGSL meeting minutes 2021-11-09
  • RM: When you see f(, you don’t know in the grammar if it’s a function call or function call followed by equals. You have to look ahead quite far. Propose grammar change accepts the same language, but removes one of the 4 conflicts in the grammar.
  • DN: Haven’t reviewed yet. In principle it’s ok.
  • JG: Would like to allow future growth. But address it then.
  • Agree to merge (after mechanical review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wgsl WebGPU Shading Language Issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants