Skip to content
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.

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

Machine readable syntax specification #3707

Merged
merged 4 commits into from
Feb 5, 2025
Merged

Machine readable syntax specification #3707

merged 4 commits into from
Feb 5, 2025

Conversation

jsproz
Copy link
Contributor

@jsproz jsproz commented Dec 2, 2024

Closes #3686

Description

Added a machine readable syntax specification for the Cadence language and a Syntax Notation document.


  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@jsproz jsproz added the Documentation Improvements or additions to documentation label Dec 2, 2024
@jsproz jsproz changed the title Sprowes/syntax Machine readable syntax specification Dec 2, 2024
@jsproz jsproz self-assigned this Dec 2, 2024
Copy link

github-actions bot commented Dec 2, 2024

Cadence Benchstat comparison

This branch with compared with the base branch onflow:master commit 24783e3
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -benchmem -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

@jsproz jsproz enabled auto-merge December 2, 2024 21:40
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice work!

docs/syntax/Syntax Notation.md Show resolved Hide resolved
docs/syntax/Cadence.syntax Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Show resolved Hide resolved
docs/syntax/Cadence.syntax Show resolved Hide resolved
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Did another pass over types as requested. Mainly the precedence rules need to be encoded properly, and the only other thing is the intersection types.

Also did another pass over all declarations, and left a few minor comments, especially regarding what types are allowed.

Will do another pass over expressions tomorrow.

docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Did another pass reviewing the expression parsing. Looks good, a couple notes.

Note that some expressions need newline differentiation to resolve ambiguities when expressions are used as statements, see the "(* if no line terminator ahead *)" comments in the expression rules of the EBNF grammar, like

| postfixExpression (* if no line terminator ahead *) invocation
.

For example:

foo(bar) // invocation
foo    // identifier expression
(bar)  // parenthesized expression

docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
docs/syntax/Cadence.syntax Outdated Show resolved Hide resolved
@jsproz jsproz requested a review from turbolent February 3, 2025 20:49
Copy link
Member

@turbolent turbolent 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! Only a couple comments to resolve and then we can merge what we have so far.

@SupunS Could you please take a final look?

@jsproz jsproz merged commit 0df2af0 into master Feb 5, 2025
12 checks passed
@jsproz jsproz mentioned this pull request Feb 5, 2025
6 tasks
| viewKeyword
;

accessKeyword:
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these keywords also tokens? i.e: shouldn't be <accessKeyword>?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Machine Readable Syntax Specification
3 participants