Machine readable syntax specification #3707

jsproz · 2024-12-02T21:37:58Z

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

…ner and parser.

github-actions · 2024-12-02T21:39:30Z

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

turbolent

Nice work!

turbolent · 2024-12-03T17:27:34Z

docs/syntax/Syntax Notation.md

+
+### Set Rule
+
+Set rules consist of a sequence of intervals separated by a comma (`,`). These intervals can either represent a single Unicode code point or a range of code points, separated by a hyphen (`-`).


Aren't alternations with | also sets? Could we maybe just consistently use | to avoid unnecessarily adding a new operator uncommon in other notions

turbolent · 2024-12-03T17:33:32Z

docs/syntax/Cadence.syntax

+    ;
+
+<identifierToken>:
+        "ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz" *identifierFollower


Is it possible to express these as ranges? e.g. [A-Z_a-z] like in flex?

turbolent · 2024-12-03T17:36:46Z

docs/syntax/Cadence.syntax

+
+*
+program:
+         *declaration( declarationSeparator ) $


Could we make it more explicit that the notation here means "separated by"? The parentheses look like grouping (like in other notations), and at first glance it looks like they are unnecessary and can be removed.

This language does not have a grouping operator.

In that case, could you please leave a comment above, with an equivalent rule in somehting like EBNF notation? Same for the comments above (adding a comment for ranges)

turbolent · 2024-12-03T17:38:24Z

docs/syntax/Cadence.syntax

+        access contractKeyword interfaceKeyword nonReservedIdentifier *1interfaceInheritance "{" membersAndNestedDeclarations "}"
+    ;
+
+enumInterfaceDeclaration:


We might want to indicate that enum interfaces are illegal, but this rule exists to produce better error messages. We might in general make this syntax definition as strict as possible, so maybe just remove the rule

I was thinking we should have two versions of the syntax. A strict grammar that is allowed by the compiler, and a more general, internal syntax that is used to produce useful error messages.

That's a good idea. To begin with we might at least indicate which variant of the syntax this is

turbolent · 2024-12-03T17:40:02Z

docs/syntax/Cadence.syntax

+        accessPriv
+    |   accessPub


pub, pub(set), and priv got removed in Cadence 1.0

This syntax is focused on duplicating the behavior of the interpreter's parser. These would produce errors in the semantic checks. That should also be part of the specification.

Maybe at least leave a comment documenting this decision, e.g. something like // Invalid since Cadence 1.0, so not part of the strict syntax

turbolent · 2024-12-03T17:41:45Z

docs/syntax/Cadence.syntax

+    ;
+
+fullTypeOptional:
+        . optional


The . looks like "any character" here, like in other notations. Maybe leave a comment

The . and _ were chosen to represent adjacent and not-adjacent. The language for the parser section does not match individual characters. It matches tokens.

Right, that's OK. It might not be obvious to readers unfamiliar with this custom notation, and given the . notation means something else in other commonly used notations, maybe help the reader here and point out that this does not mean "any character" but "adjacent"

turbolent · 2024-12-03T17:43:17Z

docs/syntax/Cadence.syntax

+        move
+    ;
+
+expression:


The precedence numbers are an implementation detail of the current parser implementation, maybe remove them from the document as they are not necessary and probably just confusing. The rules are already structured in a way where they encode the precedence rules.

That was the intention. They were there only to help define the grammar. I forgot to remove them.

turbolent

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.

turbolent · 2024-12-09T23:01:26Z

docs/syntax/Cadence.syntax

+    ;
+
+functionDeclaration:
+        access *1viewKeyword funKeyword nonReservedIdentifier parameterList *1functionDeclarationTypeAnnotation *1functionBlock


Here and above: The functionBlock has an "optional" multiplicity here, but this is only the case for some situations, e.g. in interface declarations. Otherwise it is required. We can leave this as-is, but maybe leave a note

turbolent · 2024-12-09T23:05:22Z

docs/syntax/Cadence.syntax

+    ;
+
+parseLabeledParameter:
+        parameterPublicName parameterLocalName ":" typeAnnotation *1defaultValue


Maybe rename parameterPublicName to argumentLabel, and parameterLocalName to just parameterName, to match the terminology we have elsewhere

turbolent · 2024-12-09T23:06:24Z

docs/syntax/Cadence.syntax

+    ;
+
+parseLabeledParameter:
+        parameterPublicName parameterLocalName ":" typeAnnotation *1defaultValue


The defaultValue is only valid in some cases, in resource destroyed event declarations. Maybe leave a note that we parse more than semantically allowed

turbolent · 2024-12-09T23:11:28Z

docs/syntax/Cadence.syntax

+
+conformances:
+        conformancesTypes
+    |   conformancesEmpty


Can this be simplified to just | empty, do we need a separate empty rule conformancesEmpty?

turbolent · 2024-12-09T23:13:15Z

docs/syntax/Cadence.syntax

+    ;
+
+structInterfaceDeclaration:
+        access structKeyword interfaceKeyword nonReservedIdentifier *1interfaceInheritance "{" membersAndNestedDeclarations "}"


How does interfaceInheritance differ from conformances? Maybe remove *1interfaceInheritance and just use conformances here and for the other interface rules below

turbolent · 2024-12-09T23:42:57Z

docs/syntax/Cadence.syntax

+
+escapedCharacter:
+        escapedUnicodeScalar
+    |   escapedCCharacter


Potential typo (extra C), maybe rename to escapedCharacter

turbolent · 2024-12-09T23:55:12Z

docs/syntax/Cadence.syntax

+    ;
+
+functionType:
+        funKeyword "(" *1functionTypeParameters ")" *1functionTypeReturn


Function types may have a view modifier

turbolent · 2024-12-10T00:01:43Z

docs/syntax/Cadence.syntax

+fullType:
+        fullTypeNormal
+    |   fullTypeNested
+    ;
+
+fullTypeNormal:
+        *1fullTypeReference innerType *fullTypeOptional
+    ;


Maybe encode the precedence rules for types in nested rules, like it is done for expressions, see the typeLeftBindingPower* constants. For example, references bind tighter than optionals

turbolent · 2024-12-10T00:09:05Z

docs/syntax/Cadence.syntax

+    |   dictionaryType
+    ;
+
+typeRestrictions:


Restriction types got removed in Cadence 1.0. Instead, there are only intersection types, which have this syntax, but not as a "suffix" to a type. For example, before a type T could be restricted with T{...}, where ... is a list of zero or more nominal types. Now there is just the syntax for intersection types {...} where ... is a list of one or more nominal types.

turbolent · 2024-12-10T00:10:55Z

docs/syntax/Cadence.syntax

+    ;
+
+interfaceInheritance:
+        ":" 1*fullType( "," )


Only nominal types are allowed

turbolent

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

cadence/docs/cadence.ebnf

Line 457 in 5364220

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

.

For example:

foo(bar) // invocation

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

turbolent · 2024-12-10T22:01:44Z

docs/syntax/Cadence.syntax

+    ;
+
+comparisonExpression:  // 50
+        1*2nilCoalescingExpression( comparisonOp )


Though type checking might reject nested comparison expressions, parsing still allows them.
So this should not have an upper bound in the multiplicity:

Suggested change

1*2nilCoalescingExpression( comparisonOp )

1*nilCoalescingExpression( comparisonOp )

turbolent · 2024-12-10T22:08:10Z

docs/syntax/Cadence.syntax

+primaryExpression:
+        nullDenotation *leftDenotation
+    ;
+
+nullDenotation:
+        identifierExpression
+    ;
+
+leftDenotation:
+        forceUnwrapOp
+    |   accessExpression
+    |   invocationExpression
+    ;


"null denotation" and "left denotation" are terminology of Pratt's Top Down Operator Precedence parsing technique, i.e. are an implementation detail of the current Go parser. They shouldn't be used as rule names.

turbolent · 2024-12-10T22:10:20Z

docs/syntax/Cadence.syntax

+        *1prefixOp postfixExpression
+    ;
+
+postfixExpression:  // 160


Force unwrapping is on this precedence level

turbolent · 2024-12-10T22:15:04Z

docs/syntax/Cadence.syntax

+leftDenotation:
+        forceUnwrapOp
+    |   accessExpression
+    |   invocationExpression
+    ;
+
+accessExpression:  // 170
+        memberExpression
+    |   indexExpression
+    ;


member expression, indexing expression, and invocation expression are all part of the same precedence and should probably be in the same rule. As noted above, force unwrapping has lower precedence (needs to be in a "higher" rule)

Joe Sprowes added 2 commits November 29, 2024 10:59

Add .syntax file for describing the Cadence programming language scan…

6f80ccf

…ner and parser.

Add syntax notation document.

b1ba6d5

jsproz added the Documentation Improvements or additions to documentation label Dec 2, 2024

jsproz requested review from turbolent and SupunS as code owners December 2, 2024 21:37

jsproz changed the title ~~Sprowes/syntax~~ Machine readable syntax specification Dec 2, 2024

jsproz self-assigned this Dec 2, 2024

jsproz enabled auto-merge December 2, 2024 21:40

turbolent reviewed Dec 3, 2024

View reviewed changes

turbolent reviewed Dec 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Machine readable syntax specification #3707

Machine readable syntax specification #3707

jsproz commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

turbolent left a comment

turbolent Dec 3, 2024

turbolent Dec 3, 2024

turbolent Dec 3, 2024

jsproz Dec 6, 2024

turbolent Dec 6, 2024

turbolent Dec 3, 2024

jsproz Dec 6, 2024

turbolent Dec 6, 2024

turbolent Dec 3, 2024

jsproz Dec 6, 2024

turbolent Dec 6, 2024 •

edited

Loading

turbolent Dec 3, 2024

jsproz Dec 6, 2024

turbolent Dec 6, 2024

turbolent Dec 3, 2024

jsproz Dec 6, 2024

turbolent left a comment

turbolent Dec 9, 2024

turbolent Dec 9, 2024 •

edited

Loading

turbolent Dec 9, 2024

turbolent Dec 9, 2024

turbolent Dec 9, 2024

turbolent Dec 9, 2024

turbolent Dec 9, 2024

turbolent Dec 10, 2024

turbolent Dec 10, 2024

turbolent Dec 10, 2024

turbolent left a comment

turbolent Dec 10, 2024

turbolent Dec 10, 2024

turbolent Dec 10, 2024

turbolent Dec 10, 2024


		### Set Rule

		Set rules consist of a sequence of intervals separated by a comma (`,`). These intervals can either represent a single Unicode code point or a range of code points, separated by a hyphen (`-`).

	1*2nilCoalescingExpression( comparisonOp )
	1*nilCoalescingExpression( comparisonOp )

Machine readable syntax specification #3707

Are you sure you want to change the base?

Machine readable syntax specification #3707

Conversation

jsproz commented Dec 2, 2024

Description

github-actions bot commented Dec 2, 2024

Cadence Benchstat comparison

turbolent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turbolent Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turbolent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turbolent Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turbolent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turbolent Dec 6, 2024 •

edited

Loading

turbolent Dec 9, 2024 •

edited

Loading