[Macros] Attached macro expansions return single string #66918
Conversation
rintaro
requested review from
hborla,
slavapestov,
xedin,
zoecarver and
CodaFi
as code owners
|
swiftlang/swift-syntax#1845 |
| case expandMacroResult( | ||
| expandedSource: String?, | ||
| diagnostics: [PluginMessage.Diagnostic] | ||
| ) |
There was a problem hiding this comment.
Another idea is, keep expandFreestandingMacroResult as-is, and make expandAttachedMacroResult be
case expandAttachedMacroResult(
expandedSource: String?,
expandedSources: [String]?,
diagnostics: [PluginMessage.Diagnostic]
)then plugins fill either expandedSource or expandedSources depending on host's protocol version.
@bnbarham wdyt?
rintaro
changed the title
rintaro
force-pushed
the
macros-plugins-collappsed-attached-result
branch
from
8fd9ccc to
e122087
Compare
|
swiftlang/swift-syntax#1845 |
rintaro
requested review from
DougGregor,
bnbarham and
ahoppen
and removed request for
xedin,
slavapestov,
CodaFi,
hborla and
zoecarver
| case expandMacroResult( | ||
| expandedSource: String?, | ||
| diagnostics: [PluginMessage.Diagnostic] | ||
| ) |
| @@ -407,6 +407,29 @@ func checkMacroDefinition( | |||
| } | |||
| } | |||
|
|
|||
| // Make an expansion result for '@_cdecl' function caller. | |||
| func makeExpansionOutputResult( | |||
There was a problem hiding this comment.
Doesn't have to be this PR, but could this use BridgedString instead?
There was a problem hiding this comment.
withBridgedString can't escape the closure argument. Since we need to send the result back to C++ somehow, and currently (afaik) we don't have a way to call C++ lambda from Swift, I don't think it's super simple to use it.
Of course we can create BridgedString using the allocated buffer instead, but I'm not sure we should do it. 🤔
There was a problem hiding this comment.
To be clear, I meant the latter + having swift_ASTGen_expand* return it instead of taking it as an output parameter. Is the concern that BridgedString is currently only ever used in the other direction at the moment? IMO having it as the return value + a comment mentioning the caller owns the underlying buffer is enough to me. But like I said, it shouldn't block this PR so we can talk about it later.
| /// Deinitialize and unset athe plugin capability stored in C++ | ||
| /// 'LoadexExeucutablePlugin'. This should be called inside lock. |
There was a problem hiding this comment.
| /// Deinitialize and unset athe plugin capability stored in C++ | |
| /// 'LoadexExeucutablePlugin'. This should be called inside lock. | |
| /// Deinitialize and unset the plugin capability stored in C++ | |
| /// 'LoadedExecutablePlugin'. This should be called inside lock. |
* 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 protcol version to see if it suppports 'macroExpansionResult', and fall back to old behavior if necessary
rintaro
force-pushed
the
macros-plugins-collappsed-attached-result
branch
from
e122087 to
eefe9dc
Compare
|
swiftlang/swift-syntax#1845 |
This is an effort of de-duplicating macro expansion codes between dylib plugins, executable plugins, and macro testing i.e.
MacroSystem.swift-syntax change: swiftlang/swift-syntax#1845
collapse(expansions:for:attachedTo:)toSwiftSyntaxMacroExpansionSwiftSyntaxMacroExpansion.expandAttachedMacro()now perform collapsingSwiftSyntaxMacroExpansion.expandAttachedMacroWithoutCollapsing()to keep old behaviorgetCapabilitynow sends the host protocol versionmacroExpansionResultthat returns single string for bothexpandFreestandingMacroandexpandAttachedMacroexpandFreestandingMacroResultandexpandAttachedMacroResultto keep compatibilitymacroExpansionResult, and fall back to old behavior if necessaryAlso:
PluginMessage.MacroRoledefinition mismatchfreeStandingDeclaration->declaration