Skip to content

Improve error messages when using SyntaxStringInterpolation with invalid syntax code #855

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

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Sep 28, 2022

If parsing source text of a SyntaxStringInterpolation produced a tree with errors or did not consume all characters in the string literal (and would thus drop text), raise a fatal error.

To make sure parsing errors are displayed at the string literal itself and don’t navigate Xcode to the SyntaxExpressibleByStringInterpolation initializer that fatalErrored, split the initializer into a throwing variant and a @_transparent one that fatalErrors when the throwing initializer encounters an error. Because @_transparent is also transparent in terms of source locations, this will make the crash appear at the start of the string literal.

Example error messages

Screenshot 2022-09-29 at 14 18 42

Screenshot 2022-09-29 at 14 20 22

@ahoppen ahoppen marked this pull request as ready for review September 29, 2022 10:28
@ahoppen ahoppen force-pushed the ahoppen/improved-errors-in-string-interpolation branch from 34df0a8 to 7ecbcd7 Compare September 29, 2022 10:28
/// are reported at the string literal itself.
/// This makes debugging easier because Xcode will jump to the string literal
/// that had a parsing error instead of the initializer that raised the `fatalError`
@_transparent
public init(stringInterpolation: SyntaxStringInterpolation) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Does anyone have any concerns about this @_transparent hack? @DougGregor

Copy link
Member

Choose a reason for hiding this comment

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

Can we do this using default arguments with #file and #line instead? @_transparent is a messy feature that we'd prefer not spread much farther.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because Xcode’s debugger will still jump to the position where the fatalError was called without @_transparent. 😬

@ahoppen ahoppen requested a review from DougGregor September 29, 2022 10:30
@ahoppen
Copy link
Member Author

ahoppen commented Sep 29, 2022

@swift-ci Please test

@ahoppen ahoppen force-pushed the ahoppen/improved-errors-in-string-interpolation branch from 7ecbcd7 to d794d41 Compare September 30, 2022 14:39
@ahoppen
Copy link
Member Author

ahoppen commented Sep 30, 2022

@swift-ci Please test

…lid syntax code

If parsing source text of a `SyntaxStringInterpolation` produced a tree with errors or did not consume all characters in the string literal (and would thus drop text), raise a fatal error.

To make sure parsing errors are displayed at the string literal itself and don’t navigate Xcode to the `SyntaxExpressibleByStringInterpolation` initializer that `fatalError`ed, split the initializer into a throwing variant and a `@_transparent` one that `fatalError`s when the throwing initializer encounters an error. Because `@_transparent` is also transparent in terms of source locations, this will make the crash appear at the start of the string literal.
@ahoppen ahoppen force-pushed the ahoppen/improved-errors-in-string-interpolation branch from d794d41 to b394a84 Compare September 30, 2022 16:34
@ahoppen
Copy link
Member Author

ahoppen commented Sep 30, 2022

@swift-ci Please test

@ahoppen ahoppen merged commit b233edb into swiftlang:main Sep 30, 2022
@ahoppen ahoppen deleted the ahoppen/improved-errors-in-string-interpolation branch September 30, 2022 21:22
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