Skip to content

[5.9] Fix for some wrong attribute fix its #1688

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

kimdv
Copy link
Contributor

@kimdv kimdv commented May 22, 2023

  • Explanation: Before the parser didn't consume a label correctly inside _originallyDefinedIn, _unavailableFromAsync and _expose. The parser tought it was an identifier so it skipped.
    Now it consumes an identifier and remap it so a string segment
  • Scope: Swift parser and diagnostics
  • Risk: Low
  • Testing: Added correct test cases
  • Issue: N/A
  • Reviewer: @ahoppen on Fix for some wrong attribute fix its #1566

@kimdv kimdv requested a review from ahoppen as a code owner May 22, 2023 12:42
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Parser change is minimal and safe. The lexer will never create an identifier lexeme inside after an open string quote in valid source code.

Comment on lines +428 to +438
DiagnosticSpec(
locationMarker: "1️⃣",
message: #"expected '"' in string literal"#,
fixIts: [#"insert '"'"#]
),
DiagnosticSpec(
locationMarker: "2️⃣",
message: #"expected '"' to end string literal"#,
fixIts: [#"insert '"'"#]
),
],
Copy link
Contributor

Choose a reason for hiding this comment

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

How difficult is it to make this a single diagnostic? Seems a little weird as two to me, ie. I would expect a surround '...' in '"'

Copy link
Member

@ahoppen ahoppen May 22, 2023

Choose a reason for hiding this comment

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

Filed #1691. We can take care of this on main, it’s not necessary to do it for Swift 5.9.

@ahoppen
Copy link
Member

ahoppen commented May 22, 2023

@swift-ci Please test

@kimdv
Copy link
Contributor Author

kimdv commented May 23, 2023

@swift-ci please test

…-of-quote-tokens

Fix for some wrong attribute fix its
@kimdv kimdv force-pushed the kimdv/cherry-pick-fix-wrong-wrapping-of-quote-tokens branch from 2327497 to eba6f40 Compare May 23, 2023 05:38
@kimdv
Copy link
Contributor Author

kimdv commented May 23, 2023

@swift-ci please test

@kimdv
Copy link
Contributor Author

kimdv commented May 23, 2023

@swift-ci please test windows

1 similar comment
@kimdv
Copy link
Contributor Author

kimdv commented May 23, 2023

@swift-ci please test windows

@ahoppen ahoppen merged commit d367cf0 into swiftlang:release/5.9 May 23, 2023
@kimdv kimdv deleted the kimdv/cherry-pick-fix-wrong-wrapping-of-quote-tokens branch May 23, 2023 16:50
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