Skip to content

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Aug 24, 2023

  • Unify forDirective with ExprFlavor, getting rid of one boolean parameter in expression parsing
  • Rename ExprFlavor.basic -> stmtCondition because this case is used primarily in statement conditions that need to disambiguate trailing closures from the statement’s body
  • Rename ExprFlavor.trailingClosure -> basic because this is the base case that we use inside normal expression parsing
  • Add argument labels to flavor to match the pattern parameter that also has a label and make parseExpression parameters required so that each caller needs to make a conscious choice about the values and so it’s easier to search for where each of the flavors is being used.

@ahoppen ahoppen requested review from rintaro and bnbarham August 24, 2023 22:11
@ahoppen
Copy link
Member Author

ahoppen commented Aug 24, 2023

@swift-ci Please test

Rename
- `basic` -> `stmtCondition` because this case is used primarily in statement conditions that need to disambiguate trailing closures from the statement’s body
- `trailingClosure` -> `basic` because this is the base case that we use inside normal expression parsing

Also add argument labels to `flavor` to match the `pattern` parameter that also has a label and make `parseExpression` parameters required so that each caller needs to make a conscious choice about the values and so it’s easier to search for where each of the flavors is being used.
@ahoppen ahoppen force-pushed the ahoppen/expr-flavor-refactoring branch from 2054b60 to 536fd81 Compare August 27, 2023 04:18
@ahoppen
Copy link
Member Author

ahoppen commented Aug 27, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Aug 27, 2023

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 43b7a9f into swiftlang:main Aug 30, 2023
@ahoppen ahoppen deleted the ahoppen/expr-flavor-refactoring branch August 30, 2023 23:40
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.

3 participants