Skip to content

WGSL grammar improvements #5461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
benbaarber wants to merge 4 commits into
base: main
Choose a base branch
from
Open

WGSL grammar improvements #5461

benbaarber wants to merge 4 commits into from

Conversation

@benbaarber
Copy link

@benbaarber benbaarber commented Nov 27, 2025 •
edited
Loading

Some improvements to the wgsl grammar that enable better tree-sitter querying.

See related PR in tree-sitter-wgsl: gpuweb/tree-sitter-wgsl#5

Added a postprocessing script (wgsl/tools/process-ts-grammar.py) to expose _template_args_start, _template_args_end, and _block_comment as named nodes (replaces occurrences of those strings with leading underscores removed). Intended to be applied to the grammar.js to put in the tree-sitter-wgsl repo.

Changes in syntax.bnf:

  • factor out shift_operator from _shift_left & _shift_right so it can be a named node
  • factor out comparison_operator from all comparison operators including >, >=, <, <=, so it can be a named node
  • replace generic identifier with type_specifier after -> in function header (better highlighting)

Other changes in extract_grammar.py:

  • rename _comment to line_comment, so it can be a named node
  • rename _reserved to reserved, same reason

@benbaarber benbaarber mentioned this pull request Nov 27, 2025
Open
@dneto0 dneto0 self-requested a review November 27, 2025 18:32
mehmetoguzderin
mehmetoguzderin requested changes Nov 28, 2025
View reviewed changes
Copy link
Member

@mehmetoguzderin mehmetoguzderin left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I had a few questions.

  1. Could you make sure CI is green? You can replicate the behavior of CI by opening the repo in devcontainer, which should provide a ready setup to test and develop.
  2. Does the name change to template start and end impact the operation of scanner?

Thanks a lot in advance.

@benbaarber
Copy link
Author

benbaarber commented Nov 28, 2025

My fault, I had only run make validate which worked, but I didnt run make to go through all build steps. Looks like removing the underscore prefix from _template_args_start and _template_args_end broke some of the bikeshed process. I've tried replacing all occurrences of those tokens with the underscores removed but i get this error:

(venv) root@dafd3fff5255:/workspaces/gpuweb/wgsl# make
bash -c ' \
  if [ -f ../tools/custom-action/dependency-versions.sh ]; then \
    source ../tools/custom-action/dependency-versions.sh; \
  else \
    source /dependency-versions.sh; \
  fi && \
  python3 ./tools/extract-grammar.py --flow x'
extract-grammar.py: Extract...
extract-grammar.py: ...Creating tree-sitter parser
source ../tools/custom-action/dependency-versions.sh && python3 ./tools/extract-grammar.py --spec index.bs --scanner ./grammar/src/scanner.c --tree-sitter-dir grammar --flow b
extract-grammar.py: Build tree_sitter_wgsl...
extract-grammar.py: ...Skip rebuilding because the grammar has not changed
DIE_ON=everything bash ../tools/invoke-bikeshed.sh index.pre.html index.bs ./grammar/src/scanner.c wgsl.recursive.bs.include wgsl.reserved.bs.include syntax/additive_expression.syntax.bs.include syntax/additive_operator.syntax.bs.include syntax/align_attr.syntax.bs.include syntax/argument_expression_list.syntax.bs.include syntax/assert_statement.syntax.bs.include syntax/assignment_statement.syntax.bs.include syntax/attribute.syntax.bs.include syntax/binary_and_expression.syntax.bs.include syntax/binary_or_expression.syntax.bs.include syntax/binary_xor_expression.syntax.bs.include syntax/binding_attr.syntax.bs.include syntax/bitwise_expression.syntax.bs.include syntax/blend_src_attr.syntax.bs.include syntax/bool_literal.syntax.bs.include syntax/break_if_statement.syntax.bs.include syntax/break_statement.syntax.bs.include syntax/builtin_attr.syntax.bs.include syntax/builtin_value_name.syntax.bs.include syntax/call_expression.syntax.bs.include syntax/call_phrase.syntax.bs.include syntax/case_clause.syntax.bs.include syntax/case_selectors.syntax.bs.include syntax/case_selector.syntax.bs.include syntax/comparison_operator.syntax.bs.include syntax/component_or_swizzle_specifier.syntax.bs.include syntax/compound_assignment_operator.syntax.bs.include syntax/compound_statement.syntax.bs.include syntax/compute_attr.syntax.bs.include syntax/const_assert.syntax.bs.include syntax/const_attr.syntax.bs.include syntax/continue_statement.syntax.bs.include syntax/continuing_compound_statement.syntax.bs.include syntax/continuing_statement.syntax.bs.include syntax/core_lhs_expression.syntax.bs.include syntax/decimal_float_literal.syntax.bs.include syntax/decimal_int_literal.syntax.bs.include syntax/decrement_statement.syntax.bs.include syntax/default_alone_clause.syntax.bs.include syntax/diagnostic_attr.syntax.bs.include syntax/diagnostic_control.syntax.bs.include syntax/diagnostic_directive.syntax.bs.include syntax/diagnostic_name_token.syntax.bs.include syntax/diagnostic_rule_name.syntax.bs.include syntax/else_clause.syntax.bs.include syntax/else_if_clause.syntax.bs.include syntax/enable_directive.syntax.bs.include syntax/enable_extension_list.syntax.bs.include syntax/enable_extension_name.syntax.bs.include syntax/expression_comma_list.syntax.bs.include syntax/expression.syntax.bs.include syntax/float_literal.syntax.bs.include syntax/for_header.syntax.bs.include syntax/for_init.syntax.bs.include syntax/for_statement.syntax.bs.include syntax/for_update.syntax.bs.include syntax/fragment_attr.syntax.bs.include syntax/func_call_statement.syntax.bs.include syntax/function_decl.syntax.bs.include syntax/function_header.syntax.bs.include syntax/global_assert.syntax.bs.include syntax/global_decl.syntax.bs.include syntax/global_directive.syntax.bs.include syntax/global_value_decl.syntax.bs.include syntax/global_variable_decl.syntax.bs.include syntax/group_attr.syntax.bs.include syntax/hex_float_literal.syntax.bs.include syntax/hex_int_literal.syntax.bs.include syntax/id_attr.syntax.bs.include syntax/ident_pattern_token.syntax.bs.include syntax/ident.syntax.bs.include syntax/if_clause.syntax.bs.include syntax/if_statement.syntax.bs.include syntax/increment_statement.syntax.bs.include syntax/interpolate_attr.syntax.bs.include syntax/interpolate_sampling_name.syntax.bs.include syntax/interpolate_type_name.syntax.bs.include syntax/int_literal.syntax.bs.include syntax/invariant_attr.syntax.bs.include syntax/language_extension_list.syntax.bs.include syntax/language_extension_name.syntax.bs.include syntax/lhs_expression.syntax.bs.include syntax/literal.syntax.bs.include syntax/location_attr.syntax.bs.include syntax/loop_statement.syntax.bs.include syntax/member_ident.syntax.bs.include syntax/multiplicative_expression.syntax.bs.include syntax/multiplicative_operator.syntax.bs.include syntax/must_use_attr.syntax.bs.include syntax/optionally_typed_ident.syntax.bs.include syntax/param_list.syntax.bs.include syntax/param.syntax.bs.include syntax/paren_expression.syntax.bs.include syntax/primary_expression.syntax.bs.include syntax/relational_expression.syntax.bs.include syntax/requires_directive.syntax.bs.include syntax/return_statement.syntax.bs.include syntax/severity_control_name.syntax.bs.include syntax/shift_expression.syntax.bs.include syntax/shift_operator.syntax.bs.include syntax/short_circuit_and_expression.syntax.bs.include syntax/short_circuit_or_expression.syntax.bs.include syntax/singular_expression.syntax.bs.include syntax/size_attr.syntax.bs.include syntax/statement.syntax.bs.include syntax/struct_body_decl.syntax.bs.include syntax/struct_decl.syntax.bs.include syntax/struct_member.syntax.bs.include syntax/switch_body.syntax.bs.include syntax/switch_clause.syntax.bs.include syntax/switch_statement.syntax.bs.include syntax/swizzle_name.syntax.bs.include syntax/template_arg_comma_list.syntax.bs.include syntax/template_arg_expression.syntax.bs.include syntax/template_elaborated_ident.syntax.bs.include syntax/template_list.syntax.bs.include syntax/translation_unit.syntax.bs.include syntax/type_alias_decl.syntax.bs.include syntax/type_specifier.syntax.bs.include syntax/unary_expression.syntax.bs.include syntax/variable_decl.syntax.bs.include syntax/variable_or_value_statement.syntax.bs.include syntax/variable_updating_statement.syntax.bs.include syntax/vertex_attr.syntax.bs.include syntax/while_statement.syntax.bs.include syntax/workgroup_size_attr.syntax.bs.include img/operators_1.mmd.svg img/uniformity_1.mmd.svg img/uniformity_2.mmd.svg img/uniformity_3b.mmd.svg img/uniformity_3.mmd.svg img/uniformity_4.mmd.svg img/uniformity_5main.mmd.svg img/uniformity_5scale.mmd.svg
LINE 4:34: No 'dfn' refs found for 'template_args_start'.
[=syntax/template_args_start=]
LINE 4:100: No 'dfn' refs found for 'template_args_end'.
[=syntax/template_args_end=]
 ✘  Did not generate, due to errors exceeding the allowed error level.
make: *** [Makefile:16: index.pre.html] Error 2

I haven't figured out exactly what is causing this, but I think it has something to do with the scanner still outputting those tokens with the leading underscore.

I'm considering just leaving the bikeshed stuff alone and adding some postprocessing to the grammar.js, finding and replacing all instances of _template_args_start with template_args_start, and same for end. I've already done this manually and it does work. What do you think?

@benbaarber
Copy link
Author

benbaarber commented Nov 28, 2025

Since deleting the leading underscore in the bnf is inconvenient, and the effects of it are only useful in the final tree-sitter grammar used in the tree-sitter-wgsl repo, i reverted those changes. I left in the shift_operator and comparison_operator definitions though, since those are just improvements that are consistent with other patterns.

As for the tree-sitter-wgsl changes, i want to add a very simple postprocessing step that just removes leading underscores from those nodes we want to make named nodes. I did try including those find and replace operations in with the extract_grammar.py script, but the wgsl.recursive.bs.include file is generated based on the grammar.js that is generated from that, so those underscore removals cause the pipeline to break. The postprocessing should be done outside this pipeline, when the grammar.js is copied to the tree-sitter-wgsl repo. Can you elaborate on this step? Do you periodically manually copy those files over to the tree-sitter-wgsl repo and push, or is it automated?

@benbaarber
Copy link
Author

benbaarber commented Nov 28, 2025 •
edited
Loading

I created the script to postprocess grammar.js. running make and calling this script on the resulting grammar.js produces the exact grammar.js file in my PR in the tree-sitter-wgsl repo. That should be it for this PR, lmk if any issues.

mehmetoguzderin
mehmetoguzderin requested changes Dec 3, 2025
View reviewed changes
wgsl/syntax/comparison_operator.syntax.bs.include
Copy link
Member

@mehmetoguzderin mehmetoguzderin Dec 3, 2025

Choose a reason for hiding this comment

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

Instead of creating a new rule for comparison operator, is it possible to keep them as before but just remove underline? This would reduce the diff of this PR and make it easier to review. It seems that removing preceding underlines from the rules uniformly could enable the use case, though it is important to make sure that scanner works well with the adjustments.

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

Reviewers

@mehmetoguzderin mehmetoguzderin mehmetoguzderin requested changes

@dneto0 dneto0 Awaiting requested review from dneto0

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@benbaarber @mehmetoguzderin