diff --git a/lib/AST/CASTBridging.cpp b/lib/AST/CASTBridging.cpp index e76012b6a0d..d7fed9a6c99 100644 --- a/lib/AST/CASTBridging.cpp +++ b/lib/AST/CASTBridging.cpp @@ -653,15 +653,26 @@ bool Plugin_sendMessage(PluginHandle handle, const BridgedData data) { auto *plugin = static_cast(handle); StringRef message(data.baseAddress, data.size); auto error = plugin->sendMessage(message); - bool hadError = bool(error); - llvm::consumeError(std::move(error)); - return hadError; + if (error) { + // FIXME: Pass the error message back to the caller. + llvm::consumeError(std::move(error)); +// llvm::handleAllErrors(std::move(error), [](const llvm::ErrorInfoBase &err) { +// llvm::errs() << err.message() << "\n"; +// }); + return true; + } + return false; } bool Plugin_waitForNextMessage(PluginHandle handle, BridgedData *out) { auto *plugin = static_cast(handle); auto result = plugin->waitForNextMessage(); if (!result) { + // FIXME: Pass the error message back to the caller. + llvm::consumeError(result.takeError()); +// llvm::handleAllErrors(result.takeError(), [](const llvm::ErrorInfoBase &err) { +// llvm::errs() << err.message() << "\n"; +// }); return true; } auto &message = result.get(); diff --git a/lib/AST/PluginRegistry.cpp b/lib/AST/PluginRegistry.cpp index 66fd98d3f7b..d1ea95687ba 100644 --- a/lib/AST/PluginRegistry.cpp +++ b/lib/AST/PluginRegistry.cpp @@ -12,6 +12,7 @@ #include "swift/AST/PluginRegistry.h" +#include "swift/Basic/Defer.h" #include "swift/Basic/LLVM.h" #include "swift/Basic/Program.h" #include "swift/Basic/Sandbox.h" @@ -21,6 +22,7 @@ #include "llvm/Support/raw_ostream.h" #include "llvm/Config/config.h" +#include #if defined(_WIN32) #define WIN32_LEAN_AND_MEAN @@ -119,7 +121,6 @@ LoadedExecutablePlugin::LoadedExecutablePlugin( outputFileDescriptor(outputFileDescriptor) {} LoadedExecutablePlugin::~LoadedExecutablePlugin() { - // Close the pipes. close(inputFileDescriptor); close(outputFileDescriptor); @@ -131,10 +132,18 @@ ssize_t LoadedExecutablePlugin::read(void *buf, size_t nbyte) const { ssize_t bytesToRead = nbyte; void *ptr = buf; +#if defined(SIGPIPE) + /// Ignore SIGPIPE while reading. + auto *old_handler = signal(SIGPIPE, SIG_IGN); + SWIFT_DEFER { signal(SIGPIPE, old_handler); }; +#endif + while (bytesToRead > 0) { ssize_t readingSize = std::min(ssize_t(INT32_MAX), bytesToRead); ssize_t readSize = ::read(inputFileDescriptor, ptr, readingSize); - if (readSize == 0) { + if (readSize <= 0) { + // 0: EOF (the plugin exited?), -1: error (e.g. broken pipe.) + // FIXME: Mark the plugin 'stale' and relaunch later. break; } ptr = static_cast(ptr) + readSize; @@ -148,10 +157,18 @@ ssize_t LoadedExecutablePlugin::write(const void *buf, size_t nbyte) const { ssize_t bytesToWrite = nbyte; const void *ptr = buf; +#if defined(SIGPIPE) + /// Ignore SIGPIPE while writing. + auto *old_handler = signal(SIGPIPE, SIG_IGN); + SWIFT_DEFER { signal(SIGPIPE, old_handler); }; +#endif + while (bytesToWrite > 0) { ssize_t writingSize = std::min(ssize_t(INT32_MAX), bytesToWrite); ssize_t writtenSize = ::write(outputFileDescriptor, ptr, writingSize); - if (writtenSize == 0) { + if (writtenSize <= 0) { + // -1: error (e.g. broken pipe,) + // FIXME: Mark the plugin 'stale' and relaunch later. break; } ptr = static_cast(ptr) + writtenSize; @@ -170,12 +187,17 @@ llvm::Error LoadedExecutablePlugin::sendMessage(llvm::StringRef message) const { uint64_t header = llvm::support::endian::byte_swap( uint64_t(size), llvm::support::endianness::little); writtenSize = write(&header, sizeof(header)); - assert(writtenSize == sizeof(header) && - "failed to write plugin message header"); + if (writtenSize != sizeof(header)) { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "failed to write plugin message header"); + } // Write message. writtenSize = write(data, size); - assert(writtenSize == ssize_t(size) && "failed to write plugin message data"); + if (writtenSize != ssize_t(size)) { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "failed to write plugin message data"); + } return llvm::Error::success(); } @@ -187,8 +209,10 @@ llvm::Expected LoadedExecutablePlugin::waitForNextMessage() const { uint64_t header; readSize = read(&header, sizeof(header)); - // FIXME: Error handling. Disconnection, etc. - assert(readSize == sizeof(header) && "failed to read plugin message header"); + if (readSize != sizeof(header)) { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "failed to read plugin message header"); + } size_t size = llvm::support::endian::read( &header, llvm::support::endianness::little); @@ -200,6 +224,10 @@ llvm::Expected LoadedExecutablePlugin::waitForNextMessage() const { while (sizeToRead > 0) { char buffer[4096]; readSize = read(buffer, std::min(sizeof(buffer), sizeToRead)); + if (readSize == 0) { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "failed to read plugin message data"); + } sizeToRead -= readSize; message.append(buffer, readSize); } diff --git a/lib/ASTGen/Sources/ASTGen/Macros.swift b/lib/ASTGen/Sources/ASTGen/Macros.swift index 7ac35ce30ca..cff7de3b3eb 100644 --- a/lib/ASTGen/Sources/ASTGen/Macros.swift +++ b/lib/ASTGen/Sources/ASTGen/Macros.swift @@ -254,30 +254,40 @@ func expandFreestandingMacroIPC( let macro = macroPtr.assumingMemoryBound(to: ExportedExecutableMacro.self).pointee + // Send the message. let message = HostToPluginMessage.expandFreestandingMacro( macro: .init(moduleName: macro.moduleName, typeName: macro.typeName, name: macroName), discriminator: discriminator, syntax: PluginMessage.Syntax(syntax: macroSyntax, in: sourceFilePtr)!) - do { let result = try macro.plugin.sendMessageAndWait(message) - switch result { - case .expandFreestandingMacroResult(var expandedSource, let diagnostics): - if !diagnostics.isEmpty { - let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: diagEnginePtr) - diagEngine.add(exportedSourceFile: sourceFilePtr) - - for diagnostic in diagnostics { - diagEngine.emit(diagnostic, messageSuffix: " (from macro '\(macroName)')") - } - } - return expandedSource - - default: - fatalError("unexpected result") + guard + case .expandFreestandingMacroResult(let expandedSource, let diagnostics) = result + else { + throw PluginError.invalidReponseKind } + + // Process the result. + if !diagnostics.isEmpty { + let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: diagEnginePtr) + diagEngine.add(exportedSourceFile: sourceFilePtr) + diagEngine.emit(diagnostics, messageSuffix: " (from macro '\(macroName)')") + } + return expandedSource + } catch let error { - fatalError("\(error)") + let srcMgr = SourceManager(cxxDiagnosticEngine: diagEnginePtr) + srcMgr.insert(sourceFilePtr) + srcMgr.diagnose( + diagnostic: .init( + node: macroSyntax, + // FIXME: This is probably a plugin communication error. + // The error might not be relevant as the diagnostic message. + message: ThrownErrorDiagnostic(message: String(describing: error)) + ), + messageSuffix: " (from macro '\(macroName)')" + ) + return nil } } @@ -580,6 +590,7 @@ func expandAttachedMacroIPC( parentDeclSyntax = nil } + // Send the message. let message = HostToPluginMessage.expandAttachedMacro( macro: .init(moduleName: macro.moduleName, typeName: macro.typeName, name: macroName), macroRole: macroRole, @@ -589,30 +600,42 @@ func expandAttachedMacroIPC( parentDeclSyntax: parentDeclSyntax) do { let result = try macro.plugin.sendMessageAndWait(message) - switch result { - case .expandAttachedMacroResult(let expandedSources, let diagnostics): - // Form the result buffer for our caller. - - if !diagnostics.isEmpty { - let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: diagEnginePtr) - diagEngine.add(exportedSourceFile: customAttrSourceFilePtr) - diagEngine.add(exportedSourceFile: declarationSourceFilePtr) - if let parentDeclSourceFilePtr = parentDeclSourceFilePtr { - diagEngine.add(exportedSourceFile: parentDeclSourceFilePtr) - } - - for diagnostic in diagnostics { - diagEngine.emit(diagnostic, messageSuffix: " (from macro '\(macroName)')") - } - - } - return expandedSources - - default: - fatalError("unexpected result") + guard + case .expandAttachedMacroResult(let expandedSources, let diagnostics) = result + else { + throw PluginError.invalidReponseKind } + + // Process the result. + if !diagnostics.isEmpty { + let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: diagEnginePtr) + diagEngine.add(exportedSourceFile: customAttrSourceFilePtr) + diagEngine.add(exportedSourceFile: declarationSourceFilePtr) + if let parentDeclSourceFilePtr = parentDeclSourceFilePtr { + diagEngine.add(exportedSourceFile: parentDeclSourceFilePtr) + } + diagEngine.emit(diagnostics, messageSuffix: " (from macro '\(macroName)')") + } + return expandedSources + } catch let error { - fatalError("\(error)") + let srcMgr = SourceManager(cxxDiagnosticEngine: diagEnginePtr) + srcMgr.insert(customAttrSourceFilePtr) + srcMgr.insert(declarationSourceFilePtr) + if let parentDeclSourceFilePtr = parentDeclSourceFilePtr { + srcMgr.insert(parentDeclSourceFilePtr) + } + // FIXME: Need to decide where to diagnose the error: + srcMgr.diagnose( + diagnostic: .init( + node: Syntax(declarationNode), + // FIXME: This is probably a plugin communication error. + // The error might not be relevant as the diagnostic message. + message: ThrownErrorDiagnostic(message: String(describing: error)) + ), + messageSuffix: " (from macro '\(macroName)')" + ) + return nil } } diff --git a/lib/ASTGen/Sources/ASTGen/PluginHost.swift b/lib/ASTGen/Sources/ASTGen/PluginHost.swift index 792f2b9460c..e0492ad6d36 100644 --- a/lib/ASTGen/Sources/ASTGen/PluginHost.swift +++ b/lib/ASTGen/Sources/ASTGen/PluginHost.swift @@ -14,7 +14,11 @@ import CASTBridging import CBasicBridging import SwiftSyntax -struct PluginError: Error {} +enum PluginError: Error { + case failedToSendMessage + case failedToReceiveMessage + case invalidReponseKind +} @_cdecl("swift_ASTGen_initializePlugin") public func _initializePlugin( @@ -50,7 +54,7 @@ struct CompilerPlugin { return Plugin_sendMessage(opaqueHandle, BridgedData(baseAddress: data.baseAddress, size: data.count)) } if hadError { - throw PluginError() + throw PluginError.failedToSendMessage } } @@ -59,7 +63,7 @@ struct CompilerPlugin { let hadError = Plugin_waitForNextMessage(opaqueHandle, &result) defer { BridgedData_free(result) } guard !hadError else { - throw PluginError() + throw PluginError.failedToReceiveMessage } let data = UnsafeBufferPointer(start: result.baseAddress, count: result.size) // // FIXME: Add -dump-plugin-message option? @@ -209,19 +213,31 @@ class PluginDiagnosticsEngine { SwiftDiagnostic_finish(diag) } + /// Emit diagnostics. + func emit( + _ diagnostics: [PluginMessage.Diagnostic], + messageSuffix: String? = nil + ) { + for diagnostic in diagnostics { + self.emit(diagnostic) + } + } + /// Produce the C++ source location for a given position based on a /// syntax node. private func cxxSourceLocation( at offset: Int, in fileName: String ) -> CxxSourceLoc? { // Find the corresponding exported source file. - guard let exportedSourceFile = exportedSourceFileByName[fileName] + guard + let exportedSourceFile = exportedSourceFileByName[fileName] else { return nil } // Compute the resulting address. - guard let bufferBaseAddress = exportedSourceFile.pointee.buffer.baseAddress + guard + let bufferBaseAddress = exportedSourceFile.pointee.buffer.baseAddress else { return nil } diff --git a/test/Macros/macro_plugin_error.swift b/test/Macros/macro_plugin_error.swift new file mode 100644 index 00000000000..fa884f54aeb --- /dev/null +++ b/test/Macros/macro_plugin_error.swift @@ -0,0 +1,56 @@ +// REQUIRES: OS=macosx + +// RUN: %empty-directory(%t) +// RUN: split-file %s %t + +// RUN: %clang \ +// RUN: -isysroot %sdk \ +// RUN: -I %swift_src_root/include \ +// RUN: -L %swift-lib-dir -l_swiftMockPlugin \ +// RUN: -Wl,-rpath,%swift-lib-dir \ +// RUN: -o %t/mock-plugin \ +// RUN: %t/plugin.c + +// RUN: %swift-target-frontend \ +// RUN: -typecheck -verify \ +// RUN: -swift-version 5 -enable-experimental-feature Macros \ +// RUN: -load-plugin-executable %t/mock-plugin#TestPlugin \ +// RUN: -dump-macro-expansions \ +// RUN: %t/test.swift + +//--- test.swift +@freestanding(expression) macro fooMacro(_: Any) -> String = #externalMacro(module: "TestPlugin", type: "FooMacro") + +func test() { + // FIXME: Should be more user friendly message. + + let _: String = #fooMacro(1) + // expected-error @-1 {{typeMismatch(swiftASTGen.PluginToHostMessage}} + let _: String = #fooMacro(2) + // expected-error @-1 {{failedToReceiveMessage}} + let _: String = #fooMacro(3) + // expected-error @-1 {{failedToSendMessage}} + //FIXME: ^ This should succeed. Error recovery is not implemented. +} + +//--- plugin.c +#include "swift-c/MockPlugin/MockPlugin.h" + +MOCK_PLUGIN([ + { + "expect": {"getCapability": {}}, + "response": {"getCapabilityResult": {"capability": {"protocolVersion": 1}}} + }, + { + "expect": {"expandFreestandingMacro": { + "macro": {"moduleName": "TestPlugin", "typeName": "FooMacro"}, + "syntax": {"kind": "expression", "source": "#fooMacro(1)"}}}, + "response": {"invalidResponse": {}} + }, + { + "expect": {"expandFreestandingMacro": { + "macro": {"moduleName": "TestPlugin", "typeName": "FooMacro"}, + "syntax": {"kind": "expression", "source": "#fooMacro(3)"}}}, + "response": {"expandFreestandingMacroResult": {"expandedSource": "3", "diagnostics": []}} + } +])