Skip to content

[Parser] When recovering from expression parsing don't stop at '{' #42214

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

Merged
merged 2 commits into from
Apr 8, 2022

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 6, 2022

When recovering from a parser error in an expression, we resumed parsing at a '{'. I assume this was because we wanted to continue inside e.g. an if-body if parsing the condition failed, but it's actually causing more issue because when parsing e.g.

expr + has - error +

functionTakesClosure {
}

we continue parsing at the { of the trailing closure, which is a completely garbage location to continue parsing.

The motivating example for this change was (in a result builder)

Text("\(island.#^COMPLETE^#)")
takeTrailingClosure {}

Here Text(…) has an error (because it contains a code completion token) and thus we skip takeTrailingClosure, effectively parsing

Text(.) {}

which the type checker wasn’t very happy with and thus refused to provide code completion. With this change, we completely drop takeTrailingClosure {}. The type checker is a lot happier with that.

rdar://78781625

@ahoppen ahoppen requested a review from hamishknight April 6, 2022 15:34
@ahoppen ahoppen force-pushed the pr/parse-recover-no-lbrace branch from 4ee4258 to 31ca777 Compare April 7, 2022 07:16
When recovering from a parser error in an expression, we resumed parsing at a '{'. I assume this was because we wanted to continue inside e.g. an if-body if parsing the condition failed, but it's actually causing more issue because when parsing e.g.

```swift
expr + has - error +

functionTakesClosure {
}
```

we continue parsing at the `{` of the trailing closure, which is a completely garbage location to continue parsing.

The motivating example for this change was (in a result builder)
```swift
Text("\(island.#^COMPLETE^#)")
takeTrailingClosure {}
```

Here `Text(…)` has an error (because it contains a code completion token) and thus we skip `takeTrailingClosure`, effectively parsing
```swift
Text(….) {}
```

which the type checker wasn’t very happy with and thus refused to provide code completion. With this change, we completely drop `takeTrailingClosure {}`. The type checker is a lot happier with that.
@ahoppen ahoppen force-pushed the pr/parse-recover-no-lbrace branch from 31ca777 to 3e71464 Compare April 7, 2022 07:21
@ahoppen ahoppen force-pushed the pr/parse-recover-no-lbrace branch from 3e71464 to 3bbffde Compare April 7, 2022 08:13
@ahoppen
Copy link
Member Author

ahoppen commented Apr 7, 2022

@swift-ci Please smoke test

2 similar comments
@ahoppen
Copy link
Member Author

ahoppen commented Apr 7, 2022

@swift-ci Please smoke test

@ahoppen
Copy link
Member Author

ahoppen commented Apr 7, 2022

@swift-ci Please smoke test

@ahoppen ahoppen merged commit f9d27b9 into swiftlang:main Apr 8, 2022
@ahoppen ahoppen deleted the pr/parse-recover-no-lbrace branch April 8, 2022 08:46
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