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

Fix snug x<y #830

Merged
merged 2 commits into from
Dec 5, 2023
Merged

Fix snug x<y #830

merged 2 commits into from
Dec 5, 2023

Conversation

STRd6
Copy link
Contributor

@STRd6 STRd6 commented Dec 5, 2023

No description provided.

Copy link
Collaborator

@edemaine edemaine 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!

I don't understand the Coveralls error... It's says a line in e.g. xnor= is now uncovered, but it should be in tests. Unless it's a xnor=b that's uncovered; could add a test.

WAssignmentOp
__ AssignmentOp
_ OperatorAssignmentOp
_? OperatorAssignmentOp
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this change allows something like x[1]xor= true or x[1]min= x[0] which doesn't look great... but I guess it's OK, looking at the new test case. Certainly can't mean anything else.

@@ -2991,23 +2990,23 @@ AssignmentOp
# This is separate from AssignmentOp because it only works in certain contexts
# (in particular, not at the beginning of a line).
OperatorAssignmentOp
Xor "=" &Whitespace _? ->
Xor "=" _? ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

This allows x xor=y which... is probably fine, just pointing it out.

# (e.g. whitespace) after "<". This does forbid 1<2 or x<y.
/<(?!\p{ID_Start}|[_$])/ ->
return "<"
"<"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kind of surprised this works! But must be an old way of avoiding ambiguity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It originally caused some trouble with class X extends A<B> but switching to enforcing ExtendsTarget as a LeftHandSideExpression fixes it by requiring parens around expressions.

@STRd6 STRd6 merged commit 4c4d0a0 into main Dec 5, 2023
2 checks passed
@STRd6 STRd6 deleted the 824 branch December 5, 2023 16:02
---
x<y
"""

Copy link
Collaborator

@edemaine edemaine Dec 5, 2023

Choose a reason for hiding this comment

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

Perhaps should also add x<y>z from #824?

Oops, already merged — I'll add it.

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

Successfully merging this pull request may close these issues.

2 participants