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

MaybeNested expressions #1295

Merged
merged 1 commit into from
Jun 25, 2024
Merged

MaybeNested expressions #1295

merged 1 commit into from
Jun 25, 2024

Conversation

edemaine
Copy link
Collaborator

Follow up to #1293 (comment)

We don't consider indented expressions that are normal expressions, only those that are expressionized statements. So x =\n if ... is considered an indented expression, but x =\n y isn't. I think this is broken.

Basically, I added a few MaybeNested...Expressions that check for a properly indented expression. And I reviewed all uses of ExtendedExpression and a few others and decided whether to add this prefix. Mostly right-hand sides of assignments get this new treatment.

However, because we fallback to not-properly-nested parsing, it's still the case that a =\n x\n y parses as a function call. As a result of this, I can't actually think up a situation where this PR affects compilation. But it makes me feel more confident that PushIndent will have the correct indentation in some future situations... but given that it doesn't matter yet, it's also plausible that this part of the PR should wait. What do you think? (Alternatively, we could try to be stricter: when the expression starts FurtherIndented, it must be properly nested. I'm not sure whether this would work well in practice though... we're maybe used to having weirdly (non)indented expressions.)

Also added missing feature of throwing a nested expression, using the (existing but renamed) MaybeParenNestedExtendedExpression. This parallels existing features for return and yield.

Also renamed MaybeIndentedType to MaybeNestedType for consistency.

Copy link
Contributor

@STRd6 STRd6 left a comment

Choose a reason for hiding this comment

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

This is worth getting in at least for the throw capability.

@edemaine edemaine merged commit e70e050 into main Jun 25, 2024
4 checks passed
@edemaine edemaine deleted the nested branch June 25, 2024 23:12
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