Skip to content

Fix issue that caused AccessorDeclSyntax initializer with header + body to be misparsed #2739

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 1 commit into from
Jul 24, 2024

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jul 18, 2024

get {} was parsed by Parser.parseDeclaration() but it doesn't parse accessors.

AccessorDeclSyntax needs a special implementation of init(header:bodyBuilder:).

rdar://131720084

@ahoppen ahoppen requested a review from rintaro July 18, 2024 14:12
@ahoppen ahoppen requested a review from bnbarham as a code owner July 18, 2024 14:12
@ahoppen
Copy link
Member Author

ahoppen commented Jul 18, 2024

@swift-ci Please test

stringInterpolation.appendInterpolation(header)
stringInterpolation.appendLiteral(" {}")
if let parsableType = Self.self as? SyntaxParseable.Type {
decl = parsableType.init(stringInterpolation: stringInterpolation).cast(DeclSyntax.self)
Copy link
Member

@rintaro rintaro Jul 20, 2024

Choose a reason for hiding this comment

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

I think upcasting here and down casting again below is a waste. Instead of branching in where Self: DeclSyntaxProtocol extension, how about making where Self: SyntaxParseable separately?

extension WithOptionalCodeBlockSyntax where Self: SyntaxParseable & DeclSyntaxProtocol {
  public init(
    _ header: SyntaxNodeString,
    @CodeBlockItemListBuilder bodyBuilder: () throws -> CodeBlockItemListSyntax
  ) throws {
    self = "\(header) {}"
    self.body = try CodeBlockSyntax(statements: bodyBuilder()))
  }
}

(& DeclSyntaxProtocol is for resolving ambiguity)

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed offline and kept the current implementation + added a comment why we need to manually construct the string interpolation.

…body to be misparsed

`get {}` was parsed by `Parser.parseDeclaration()` but it doesn't parse accessors.

`AccessorDeclSyntax` needs a special implementation of `init(header:bodyBuilder:)`.

rdar://131720084
@ahoppen ahoppen force-pushed the accessor-decl-header-initializer branch from c40123f to 81091c1 Compare July 23, 2024 23:11
@ahoppen
Copy link
Member Author

ahoppen commented Jul 23, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Jul 24, 2024

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 69909d6 into swiftlang:main Jul 24, 2024
3 checks passed
@ahoppen ahoppen deleted the accessor-decl-header-initializer branch July 24, 2024 20:31
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