Skip to content

[Macros] Attached macro expansions return single string #1845

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

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Jun 25, 2023

swift-syntax change swiftlang/swift#66918

  • Move collapse(expansions:for:attachedTo:) to SwiftSyntaxMacroExpansion
  • SwiftSyntaxMacroExpansion.expandAttachedMacro() now perform collapsing
  • SwiftSyntaxMacroExpansion.expandAttachedMacroWithoutCollapsing() to keep old behavior
  • IPC request getCapability now sends the host protocol version
  • Unified IPC response macroExpansionResult that returns single string for both expandFreestandingMacro and expandAttachedMacro
  • Compiler accepts old expandFreestandingMacroResult and expandAttachedMacroResult to keep compatibility
  • Plugins check the compiler's protocol version to see if it suppports macroExpansionResult, and fall back to old behavior if necessary

Also:

  • Move checkMacroDefinition() to SwiftSyntaxMacroExpansion module

@rintaro
Copy link
Member Author

rintaro commented Jun 25, 2023

swiftlang/swift#66918
@swift-ci Please test

@rintaro rintaro changed the title [Macros] Attached macro expasnions return single string [Macros] Attached macro expansions return single string Jun 25, 2023
@rintaro rintaro force-pushed the macros-plugins-collappsed-attached-result branch 2 times, most recently from a8f456a to 3729244 Compare June 25, 2023 20:10
@rintaro
Copy link
Member Author

rintaro commented Jun 25, 2023

swiftlang/swift#66918
@swift-ci Please test

@kimdv
Copy link
Contributor

kimdv commented Jun 26, 2023

swiftlang/swift#66918
@swift-ci Please test windows

@rintaro rintaro requested a review from bnbarham June 26, 2023 15:48
attributeNode: attributeNode,
declarationNode: declarationNode,
parentDeclNode: parentDeclNode,
in: context
)
guard let expansions = expansions else {
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO I'd just have this check in the if below, ie.

if let expansions, hostCapability.hasExpandMacroResult {
...
} else {
...
}


// Create an "oldest" capability.
init() {
self.protocolVersion = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the default be 4?

Copy link
Member Author

Choose a reason for hiding this comment

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

4 doesn't mean anything in practice. If we use 4 as default, we should add doc-comment explaining that. I don't think it's worth it.

@rintaro rintaro force-pushed the macros-plugins-collappsed-attached-result branch from 3729244 to 23972ec Compare June 26, 2023 18:05
rintaro added 2 commits June 26, 2023 11:07
* Move collapse(expansions:for:attachedTo:) to SwiftSyntaxMacroExpansion
* SwiftSyntaxMacroExpansion.expandAttachedMacro() now perform collapsing
* SwiftSyntaxMacroExpansion.expandAttachedMacroWithoutCollapsing() for
  to keep old behavior
* IPC message 'getCapability' now send the host protocol version
* Unified IPC response 'macroExpansionResult' that returns single string
  for both 'expandFreestandingMacro' and 'expandAttachedMacro'
* Compiler accepts old 'expandFreestandingMacroResult' and
  'expandAttachedMacroResult' to keep compatibility
* Plugins check the compiler's protcol version to see if it suppports
  'macroExpansionResult', and fall back to old behavior if necessary
Macro deifnition checking is only meant to be used by something that
expands macros (e.g. compiler). Not for macro implementation.
@rintaro rintaro force-pushed the macros-plugins-collappsed-attached-result branch from 23972ec to 6fc14cb Compare June 26, 2023 18:08
@rintaro
Copy link
Member Author

rintaro commented Jun 26, 2023

swiftlang/swift#66918
@swift-ci Please test

@rintaro
Copy link
Member Author

rintaro commented Jun 26, 2023

swiftlang/swift#66918
@swift-ci Please test Windows

@rintaro rintaro merged commit 824b318 into swiftlang:main Jun 26, 2023
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.

4 participants