Consistent operator section semantics (and enable unused operator section warning) #14117
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… operator sections (which was implemented in #13920). `OprSectionBoundary` nodes implemented a designed behaviour that was never put into practice. The nodes were ignored in the frontend and the backend, where an operator section has always included exactly the operator application. This change brings the parser's AST in line with current semantics. Also: - The operand of `UnaryOprApp` is no longer optional in the AST types. The parser already did not construct any `UnaryOprApp` without an operand present; the types now reflect this.
kazcw
added
the
CI: No changelog needed
kazcw
requested review from
4e6,
AdRiley,
Akirathan,
Frizi,
GregoryTravis,
JaroslavTulach,
PabloBuchu,
farmaazon,
hubertp,
indiv0,
jdunkerley,
somebody1234 and
vitvakatu
as code owners
vitvakatu
approved these changes
Oct 7, 2025
docs/syntax/functions.md
Outdated
| works as follows: | ||
|
|
||
| - When a binary operator is referred to without providing arguments, its value | ||
| is a method applying the operation two its two arguments (left followed by |
There was a problem hiding this comment.
Suggested change
| is a method applying the operation two its two arguments (left followed by | |
| is a method applying the operation to its two arguments (left followed by |
There was a problem hiding this comment.
I feel it can be rewritten as “When a binary operator is referred to without providing arguments, its value is a function of two arguments (left operand followed by the right one)”. It is not clear to me why here we use “method”, and in the next sentence we use “function”.
| case RawAst.Tree.Type.OprSectionBoundary: | ||
| // This expression type is not yet consistent with the backend's semantics. | ||
| // The frontend can ignore it, avoiding some problems with expressions sharing spans | ||
| // (which makes it impossible to give assign unique IDs in the current IdMap format). |
There was a problem hiding this comment.
This comment is now about TemplateFunction, right?
JaroslavTulach
approved these changes
Oct 9, 2025
| [1, 2, 3].map function=/1 . should_equal [1.0, 2.0, 3.0] | ||
| [1, 2, 3].map 1/ . should_equal [1.0, 0.5, 0.3333333333333333] | ||
| [1, 2, 3].map /1 . should_equal [1.0, 2.0, 3.0] | ||
| ``` |
There was a problem hiding this comment.
Thanks for modifying the documentation!
| yield tree; | ||
| } | ||
| case Tree.UnaryOprApp app -> app.getRhs(); | ||
| case Tree.OprSectionBoundary section -> section.getAst(); |
There was a problem hiding this comment.
OK, looks like a simplification.
| r4.pretty . should_equal "Table.new [['foo', [1, 2, 3]], ['bar', [False, True, False]], ['date', [Date.new 2022 8 27, Date.new 1999 1 1, Date.new 2012 1 23]], ['time', [Time_Of_Day.new 18, Time_Of_Day.new 1 2 34, Time_Of_Day.new 12]]]" | ||
| Debug.eval r4.pretty . should_equal r4 | ||
|
|
||
| group_builder.specify "allows setting the column type" <| |
There was a problem hiding this comment.
Great if the new syntax can discover errors like this!
Akirathan
approved these changes
Oct 9, 2025
…opr-sections # Conflicts: # docs/syntax/functions.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove operator section boundary AST nodes; enable warning for unused operator sections, which was implemented in #13920 (fixes #11203).
OprSectionBoundarynodes, and the logic for placing them, implemented a designed behaviour that was never put into practice. The backend (and frontend) were never updated to make use of the nodes, where an operator section has always included exactly the operator application. Given other changes that have been made to the usage of operator sections, the existing semantics are probably preferable. This change brings the parser's AST in line with current semantics.Also:
UnaryOprAppis no longer optional in the AST types. The parser already did not construct anyUnaryOprAppwithout an operand present; the types now reflect this.Pull Request Description
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.