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

docs: Clarify associativity of operators. #9170

Merged
merged 8 commits into from
Apr 6, 2024

Conversation

elenakrittik
Copy link
Contributor

Clarifies operator associativity as per this comment on rocket.chat.

@AThousandShips AThousandShips added enhancement topic:gdscript area:manual Issues and PRs related to the Manual/Tutorials section of the documentation cherrypick:4.1 cherrypick:4.2 labels Apr 1, 2024
@AThousandShips
Copy link
Member

As I suggested in chat I'd put the note at the top of the list, after "the following is", to make sure it's visible, and then remove the note from the ** entry as it's redundant, but keep the note in part you've edited here

@elenakrittik
Copy link
Contributor Author

As I suggested in chat I'd put the note at the top of the list, after "the following is", to make sure it's visible, and then remove the note from the ** entry as it's redundant, but keep the note in part you've edited here

Sorry, didn't read your message carefully enough!

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

LGTM, unsure if it should be in the note or not but I think it makes sense at the top and clear, might put the details below and just the mention here

@AThousandShips AThousandShips requested a review from a team April 1, 2024 14:42
@@ -224,7 +224,9 @@ in case you want to take a look under the hood.
Operators
~~~~~~~~~

The following is the list of supported operators and their precedence.
The following is the list of supported operators and their precedence. All operators are `left-associative <https://en.wikipedia.org/wiki/Operator_associativity>`_,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The following is the list of supported operators and their precedence. All operators are `left-associative <https://en.wikipedia.org/wiki/Operator_associativity>`_,
The following is the list of supported operators and their precedence. All binary operators are `left-associative <https://en.wikipedia.org/wiki/Operator_associativity>`_,

Binary operators use a table (get_rule()), which does not provide the ability to specify associativity. Unary and ternary operators call parse_precedence() in place, so we need to check their associativity. It looks like the ternary operator is right-associative:

func test():
    1 if 2 else 3 if 4 else 5
    1 if 2 else (3 if 4 else 5)
    (1 if 2 else 3) if 4 else 5
Class <unnamed> :
|   Function test(  ) :
|   |   (1) IF (2) ELSE ((3) IF (4) ELSE (5))
|   |   (1) IF (2) ELSE ((3) IF (4) ELSE (5))
|   |   ((1) IF (2) ELSE (3)) IF (4) ELSE (5)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found the sources:

Was able to confirm your observation about ternary operator, but i got really confused when i tried to determine associativity of unary operators. First of all, there is no code (or at least i think so) explicitly handling associativity, only precedence. But even then, it doesn't look like precedence is considered for unary operators because both -1 + 1 (lower precedence than unary -) and -1 ** 1 (higher precedence than -) are parsed the same ((-1) + 1 and (-1) ** 1 accordingly), so i'm not sure what exactly precedence does in this case. And in general, i struggle to understand what "associativity" even means in case of unary operators, because parsing them the way it's done right now is the only way that makes sense to me (-1 + 1 == -(1 + 1)? this would be unreasonable; neither it's possible to chain unary ops like binary ops). I'm absolutely lost on the theoretical part here 🤷

But anyway, back from crying into others' jackets. I tested all unary operators with two cases (<unary op> <expr> <binary op with higher precedence> <expr> and the same with lower precedence binary op) and in all cases it results in (<unary op> <expr>) <binary op> (<expr>), so from what i know this means all unary ops are left-associative as well?

Copy link
Member

Choose a reason for hiding this comment

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

TLDR: I think we can just write that all binary operators in GDScript are left-associative, the ternary operator is right-associative, and leave out the unary operators.

Indeed, in the case of unary operations, the definition of associativity is not obvious. I see two approaches: 1. the binding strength between operators and operands, 2. the order of code execution.

Different operators have different binding strengths. If there are operators with equal precedence to the left and right of the operand, then the order is determined using associativity. The following options are possible:

Left Right
(... infix operand) infix ... ... infix (operand infix ...)
(... infix operand) postfix ... infix (operand postfix)
(prefix operand) infix ... prefix (operand infix ...)
(prefix operand) postfix prefix (operand postfix)

Some similar operators are grouped, have the same precedence and associativity: + and -, * and /. But unary operators are too different, so have different precedence. GDScript does not have prefix/postfix increment/decrement operators (++ and --), they would be a good example to illustrate. So we simply don't have a case where associativity is important for different unary operators.

And the same unary operator cannot appear on both the left and right (unlike infix binary operators): prefix operators are always located on the left, and postfix operators on the right. 1 + 2 + 3 is valid, but ~x~ is invalid.

If we consider associativity not as the binding strength, but as the order of code execution, then there are several nuances:

  1. Prefix operators can be considered right-associative (--x is -(-x)), and postfix operators left-associative (a[x][y] is (a[x])[y]).
  2. The ternary operator and the AND and OR operators do not always evaluate all of their operands (short-circuit). However, this does not affect the order.
  3. In some languages, the order of expression execution is not always guaranteed. The compiler can make optimizations such as move common subexpressions.

Finally, in some cases, operators can be non-associative, such as chain comparisons in Python.

@dalexeev
Copy link
Member

dalexeev commented Apr 4, 2024

I think it's better not to change the table layout, just add a note before the table.

I think we can just write that all binary operators in GDScript are left-associative, the ternary operator is right-associative, and leave out the unary operators.

@elenakrittik
Copy link
Contributor Author

I think it's better not to change the table layout, just add a note before the table.

I think we can just write that all binary operators in GDScript are left-associative, the ternary operator is right-associative, and leave out the unary operators.

Hopefully good now! I keep forgetting things all around

@mhilbrunner mhilbrunner requested a review from dalexeev April 6, 2024 13:53
@mhilbrunner mhilbrunner merged commit 2925c83 into godotengine:master Apr 6, 2024
1 check passed
@mhilbrunner
Copy link
Member

Merged. Thank you! 🎉

@elenakrittik elenakrittik deleted the docs/op-assoc branch April 6, 2024 17:36
mhilbrunner pushed a commit that referenced this pull request Jul 24, 2024
* docs: Clarify associativity of operators.

(cherry picked from commit 2925c83)
@mhilbrunner
Copy link
Member

Cherry-picked to 4.2 in #9648.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation cherrypick:4.1 enhancement topic:gdscript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants