Skip to content

[Macros] Attached macro expansions return single string #66918

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.

Sign up for GitHub

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
Jun 26, 2023
Merged

[Macros] Attached macro expansions return single string #66918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/swift/AST/CASTBridging.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ typedef const void *PluginCapabilityPtr;

/// Set a capability data to the plugin object. Since the data is just a opaque
/// pointer, it's not used in AST at all.
void Plugin_setCapability(PluginHandle handle, PluginCapabilityPtr data);
void Plugin_setCapability(PluginHandle handle, PluginCapabilityPtr _Nullable data);

/// Get a capability data set by \c Plugin_setCapability .
PluginCapabilityPtr _Nullable Plugin_getCapability(PluginHandle handle);
Expand Down
172 changes: 70 additions & 102 deletions lib/ASTGen/Sources/ASTGen/Macros.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,29 @@ func checkMacroDefinition(
}
}

// Make an expansion result for '@_cdecl' function caller.
func makeExpansionOutputResult(
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't have to be this PR, but could this use BridgedString instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

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. 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

expandedSource: String?,
outputPointer: UnsafeMutablePointer<UnsafePointer<UInt8>?>,
outputLength: UnsafeMutablePointer<Int>
) -> Int {
guard var expandedSource = expandedSource else {
return -1
}

// Form the result buffer for our caller.
expandedSource.withUTF8 { utf8 in
let evaluatedResultPtr = UnsafeMutablePointer<UInt8>.allocate(capacity: utf8.count + 1)
if let baseAddress = utf8.baseAddress {
evaluatedResultPtr.initialize(from: baseAddress, count: utf8.count)
}
evaluatedResultPtr[utf8.count] = 0

outputPointer.pointee = UnsafePointer(evaluatedResultPtr)
outputLength.pointee = utf8.count
}
return 0
}

@_cdecl("swift_ASTGen_expandFreestandingMacro")
@usableFromInline
Expand Down Expand Up @@ -470,23 +493,11 @@ func expandFreestandingMacro(
discriminator: discriminator)
}

guard var expandedSource = expandedSource else {
return -1
}

// Form the result buffer for our caller.
expandedSource.withUTF8 { utf8 in
let evaluatedResultPtr = UnsafeMutablePointer<UInt8>.allocate(capacity: utf8.count + 1)
if let baseAddress = utf8.baseAddress {
evaluatedResultPtr.initialize(from: baseAddress, count: utf8.count)
}
evaluatedResultPtr[utf8.count] = 0

expandedSourcePointer.pointee = UnsafePointer(evaluatedResultPtr)
expandedSourceLength.pointee = utf8.count
}

return 0
return makeExpansionOutputResult(
expandedSource: expandedSource,
outputPointer: expandedSourcePointer,
outputLength: expandedSourceLength
)
}

func expandFreestandingMacroIPC(
Expand Down Expand Up @@ -516,7 +527,7 @@ func expandFreestandingMacroIPC(
preconditionFailure("unhandled macro role for freestanding macro")

case .expression: pluginMacroRole = .expression
case .declaration: pluginMacroRole = .freeStandingDeclaration
case .declaration: pluginMacroRole = .declaration
case .codeItem: pluginMacroRole = .codeItem
}

Expand All @@ -528,9 +539,15 @@ func expandFreestandingMacroIPC(
syntax: PluginMessage.Syntax(syntax: Syntax(expansionSyntax), in: sourceFilePtr)!)
do {
let result = try macro.plugin.sendMessageAndWait(message)
guard
case .expandFreestandingMacroResult(let expandedSource, let diagnostics) = result
else {
let expandedSource: String?
let diagnostics: [PluginMessage.Diagnostic]
switch result {
case
.expandMacroResult(let _expandedSource, let _diagnostics),
.expandFreestandingMacroResult(let _expandedSource, let _diagnostics):
expandedSource = _expandedSource
diagnostics = _diagnostics
default:
throw PluginError.invalidReponseKind
}

Expand Down Expand Up @@ -737,10 +754,10 @@ func expandAttachedMacro(
)
let discriminator = String(decoding: discriminatorBuffer, as: UTF8.self)

let expandedSources: [String]?
let expandedSource: String?
switch MacroPluginKind(rawValue: macroKind)! {
case .Executable:
expandedSources = expandAttachedMacroIPC(
expandedSource = expandAttachedMacroIPC(
diagEnginePtr: diagEnginePtr,
macroPtr: macroPtr,
rawMacroRole: rawMacroRole,
Expand All @@ -752,7 +769,7 @@ func expandAttachedMacro(
parentDeclSourceFilePtr: parentDeclSourceFilePtr,
parentDeclNode: parentDeclNode)
case .InProcess:
expandedSources = expandAttachedMacroInProcess(
expandedSource = expandAttachedMacroInProcess(
diagEnginePtr: diagEnginePtr,
macroPtr: macroPtr,
rawMacroRole: rawMacroRole,
Expand All @@ -765,26 +782,11 @@ func expandAttachedMacro(
parentDeclNode: parentDeclNode)
}

guard let expandedSources = expandedSources else {
return -1
}

// Fixup the source.
var expandedSource: String = collapse(expansions: expandedSources, for: MacroRole(rawMacroRole: rawMacroRole), attachedTo: declarationNode)

// Form the result buffer for our caller.
expandedSource.withUTF8 { utf8 in
let evaluatedResultPtr = UnsafeMutablePointer<UInt8>.allocate(capacity: utf8.count + 1)
if let baseAddress = utf8.baseAddress {
evaluatedResultPtr.initialize(from: baseAddress, count: utf8.count)
}
evaluatedResultPtr[utf8.count] = 0

expandedSourcePointer.pointee = UnsafePointer(evaluatedResultPtr)
expandedSourceLength.pointee = utf8.count
}

return 0
return makeExpansionOutputResult(
expandedSource: expandedSource,
outputPointer: expandedSourcePointer,
outputLength: expandedSourceLength
)
}

func expandAttachedMacroIPC(
Expand All @@ -798,7 +800,7 @@ func expandAttachedMacroIPC(
attachedTo declarationNode: DeclSyntax,
parentDeclSourceFilePtr: UnsafePointer<ExportedSourceFile>?,
parentDeclNode: DeclSyntax?
) -> [String]? {
) -> String? {
let macroName: String = customAttrNode.attributeName.description
let macro = macroPtr.assumingMemoryBound(to: ExportedExecutableMacro.self).pointee

Expand Down Expand Up @@ -840,10 +842,27 @@ func expandAttachedMacroIPC(
declSyntax: declSyntax,
parentDeclSyntax: parentDeclSyntax)
do {
let result = try macro.plugin.sendMessageAndWait(message)
guard
case .expandAttachedMacroResult(let expandedSources, let diagnostics) = result
else {
let expandedSource: String?
let diagnostics: [PluginMessage.Diagnostic]
switch try macro.plugin.sendMessageAndWait(message) {
case .expandMacroResult(let _expandedSource, let _diagnostics):
expandedSource = _expandedSource
diagnostics = _diagnostics

// Handle legacy result message.
case .expandAttachedMacroResult(let _expandedSources, let _diagnostics):
if let _expandedSources = _expandedSources {
expandedSource = SwiftSyntaxMacroExpansion.collapse(
expansions: _expandedSources,
for: MacroRole(rawMacroRole: rawMacroRole),
attachedTo: declarationNode
)
} else {
expandedSource = nil
}
diagnostics = _diagnostics
break
default:
throw PluginError.invalidReponseKind
}

Expand All @@ -857,7 +876,7 @@ func expandAttachedMacroIPC(
}
diagEngine.emit(diagnostics, messageSuffix: " (from macro '\(macroName)')")
}
return expandedSources
return expandedSource

} catch let error {
let srcMgr = SourceManager(cxxDiagnosticEngine: diagEnginePtr)
Expand Down Expand Up @@ -891,7 +910,7 @@ func expandAttachedMacroInProcess(
attachedTo declarationNode: DeclSyntax,
parentDeclSourceFilePtr: UnsafePointer<ExportedSourceFile>?,
parentDeclNode: DeclSyntax?
) -> [String]? {
) -> String? {
// Get the macro.
let macroPtr = macroPtr.bindMemory(to: ExportedMacro.self, capacity: 1)
let macro = macroPtr.pointee.macro
Expand Down Expand Up @@ -955,54 +974,3 @@ fileprivate extension SyntaxProtocol {
return formatted.trimmedDescription(matching: { $0.isWhitespace })
}
}

fileprivate func collapse<Node: SyntaxProtocol>(
expansions: [String],
for role: MacroRole,
attachedTo declarationNode: Node
) -> String {
if expansions.isEmpty {
return ""
}

var expansions = expansions
var separator: String = "\n\n"

if role == .accessor,
let varDecl = declarationNode.as(VariableDeclSyntax.self),
let binding = varDecl.bindings.first,
binding.accessor == nil {
let indentation = String(repeating: " ", count: 4)

expansions = expansions.map({ indent($0, with: indentation) })
expansions[0] = "{\n" + expansions[0]
expansions[expansions.count - 1] += "\n}"
} else if role == .memberAttribute {
separator = " "
}

return expansions.joined(separator: separator)
}

fileprivate func indent(_ source: String, with indentation: String) -> String {
if source.isEmpty || indentation.isEmpty {
return source
}

var indented = ""
var remaining = source[...]
while let nextNewline = remaining.firstIndex(where: { $0.isNewline }) {
if nextNewline != remaining.startIndex {
indented += indentation
}
indented += remaining[...nextNewline]
remaining = remaining[remaining.index(after: nextNewline)...]
}

if !remaining.isEmpty {
indented += indentation
indented += remaining
}

return indented
}
30 changes: 22 additions & 8 deletions lib/ASTGen/Sources/ASTGen/PluginHost.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,24 +154,38 @@ struct CompilerPlugin {

/// Initialize the plugin. This should be called inside lock.
func initialize() throws {
// Get capability.
let response = try self.sendMessageAndWaitWithoutLock(.getCapability)
// Send host capability and get plugin capability.
let hostCapability = PluginMessage.HostCapability(
protocolVersion: PluginMessage.PROTOCOL_VERSION_NUMBER
)
let request = HostToPluginMessage.getCapability(capability: hostCapability)
let response = try self.sendMessageAndWaitWithoutLock(request)
guard case .getCapabilityResult(let capability) = response else {
throw PluginError.invalidReponseKind
}

deinitializePluginCapabilityIfExist()

let ptr = UnsafeMutablePointer<Capability>.allocate(capacity: 1)
ptr.initialize(to: .init(capability))
Plugin_setCapability(opaqueHandle, UnsafeRawPointer(ptr))
}

/// Deinitialize and unset the plugin capability stored in C++
/// 'LoadedExecutablePlugin'. This should be called inside lock.
func deinitializePluginCapabilityIfExist() {
if let ptr = Plugin_getCapability(opaqueHandle) {
let capabilityPtr = UnsafeMutableRawPointer(mutating: ptr)
.assumingMemoryBound(to: PluginMessage.PluginCapability.self)
capabilityPtr.deinitialize(count: 1)
capabilityPtr.deallocate()
Plugin_setCapability(opaqueHandle, nil)
}
}

func deinitialize() {
self.withLock {
if let ptr = Plugin_getCapability(opaqueHandle) {
let capabilityPtr = UnsafeMutableRawPointer(mutating: ptr)
.assumingMemoryBound(to: PluginMessage.PluginCapability.self)
capabilityPtr.deinitialize(count: 1)
capabilityPtr.deallocate()
}
deinitializePluginCapabilityIfExist()
}
}

Expand Down
22 changes: 18 additions & 4 deletions lib/ASTGen/Sources/ASTGen/PluginMessages.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
// NOTE: Types in this file should be self-contained and should not depend on any non-stdlib types.

internal enum HostToPluginMessage: Codable {
/// Get capability of this plugin.
case getCapability
/// Send capability of the host, and get capability of the plugin.
case getCapability(
capability: PluginMessage.HostCapability?
)

/// Expand a '@freestanding' macro.
case expandFreestandingMacro(
Expand Down Expand Up @@ -49,11 +51,19 @@ internal enum PluginToHostMessage: Codable {
capability: PluginMessage.PluginCapability
)

/// Unified response for freestanding/attached macro expansion.
case expandMacroResult(
expandedSource: String?,
diagnostics: [PluginMessage.Diagnostic]
)
Comment on lines +55 to +58
Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the new request


// @available(*, deprecated: "use expandMacroResult() instead")
case expandFreestandingMacroResult(
expandedSource: String?,
diagnostics: [PluginMessage.Diagnostic]
)

// @available(*, deprecated: "use expandMacroResult() instead")
case expandAttachedMacroResult(
expandedSources: [String]?,
diagnostics: [PluginMessage.Diagnostic]
Expand All @@ -66,7 +76,11 @@ internal enum PluginToHostMessage: Codable {
}

/*namespace*/ internal enum PluginMessage {
static var PROTOCOL_VERSION_NUMBER: Int { 4 } // Added 'loadPluginLibrary'.
static var PROTOCOL_VERSION_NUMBER: Int { 5 } // Added 'expandMacroResult'.

struct HostCapability: Codable {
var protocolVersion: Int
}

struct PluginCapability: Codable {
var protocolVersion: Int
Expand All @@ -86,7 +100,7 @@ internal enum PluginToHostMessage: Codable {

enum MacroRole: String, Codable {
case expression
case freeStandingDeclaration
case declaration
case accessor
case memberAttribute
case member
Expand Down
9 changes: 9 additions & 0 deletions lib/Sema/TypeCheckMacros.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,15 @@ initializeExecutablePlugin(ASTContext &ctx,
if (!swift_ASTGen_initializePlugin(executablePlugin, &ctx.Diags)) {
return nullptr;
}

// Resend the compiler capability on reconnect.
auto *callback = new std::function<void(void)>(
[executablePlugin]() {
(void)swift_ASTGen_initializePlugin(
executablePlugin, /*diags=*/nullptr);
});
executablePlugin->addOnReconnect(callback);

executablePlugin->setCleanup([executablePlugin] {
swift_ASTGen_deinitializePlugin(executablePlugin);
});
Expand Down
2 changes: 1 addition & 1 deletion test/Macros/macro_plugin_basic.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

// RUN: %FileCheck -strict-whitespace %s < %t/macro-expansions.txt

// CHECK: ->(plugin:[[#PID:]]) {"getCapability":{}}
// CHECK: ->(plugin:[[#PID:]]) {"getCapability":{"capability":{"protocolVersion":[[#PROTOCOL_VERSION:]]}}}
// CHECK: <-(plugin:[[#PID]]) {"getCapabilityResult":{"capability":{"protocolVersion":1}}}
// CHECK: ->(plugin:[[#PID]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"testString","typeName":"TestStringMacro"},"macroRole":"expression","syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":5,"offset":301},"source":"#testString(123)"}}}
// CHECK: <-(plugin:[[#PID]]) {"expandFreestandingMacroResult":{"diagnostics":[],"expandedSource":"\"123\"\n + \"foo \""}}
Expand Down
Loading