[Macros] Improve error handling for plugin errors

Author: rintaro

include/swift/AST/CASTBridging.h

@@ -313,6 +313,9 @@ void Plugin_setCapability(PluginHandle handle, PluginCapabilityPtr data);
 /// Get a capability data set by \c Plugin_setCapability .
 PluginCapabilityPtr _Nullable Plugin_getCapability(PluginHandle handle);
 
+/// Get the executable file path of the plugin.
+const char *Plugin_getExecutableFilePath(PluginHandle handle);
+
 /// Lock the plugin. Clients should lock it during sending and recving the
 /// response.
 void Plugin_lock(PluginHandle handle);

include/swift/AST/PluginRegistry.h

@@ -12,6 +12,7 @@
 #ifndef SWIFT_PLUGIN_REGISTRY_H
 #define SWIFT_PLUGIN_REGISTRY_H
 
+#include "swift/Basic/StringExtras.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -139,6 +140,10 @@ class LoadedExecutablePlugin {
 
   llvm::sys::procid_t getPid() { return Process->pid; }
 
+  NullTerminatedStringRef getExecutablePath() {
+    return {ExecutablePath.c_str(), ExecutablePath.size()};
+  }
+
   const void *getCapability() { return capability; };
   void setCapability(const void *newValue) { capability = newValue; };
 

lib/AST/CASTBridging.cpp

@@ -642,6 +642,11 @@ void Plugin_setCapability(PluginHandle handle, PluginCapabilityPtr data) {
   plugin->setCapability(data);
 }
 
+const char *Plugin_getExecutableFilePath(PluginHandle handle) {
+  auto *plugin = static_cast<LoadedExecutablePlugin *>(handle);
+  return plugin->getExecutablePath().data();
+}
+
 void Plugin_lock(PluginHandle handle) {
   auto *plugin = static_cast<LoadedExecutablePlugin *>(handle);
   plugin->lock();

lib/ASTGen/Sources/ASTGen/PluginHost.swift

@@ -15,11 +15,13 @@ import CBasicBridging
 import SwiftSyntax
 import swiftLLVMJSON
 
-enum PluginError: Error {
-  case stalePlugin
-  case failedToSendMessage
-  case failedToReceiveMessage
-  case invalidReponseKind
+enum PluginError: String, Error, CustomStringConvertible {
+  case stalePlugin = "plugin is stale"
+  case failedToSendMessage = "failed to send request to plugin"
+  case failedToReceiveMessage = "failed to receive result from plugin"
+  case invalidReponseKind = "plugin returned invalid result"
+
+  var description: String { rawValue }
 }
 
 @_cdecl("swift_ASTGen_initializePlugin")
@@ -28,12 +30,15 @@ public func _initializePlugin(
   cxxDiagnosticEngine: UnsafeMutablePointer<UInt8>?
 ) -> Bool {
   let plugin = CompilerPlugin(opaqueHandle: opaqueHandle)
+  let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: cxxDiagnosticEngine)
+
   do {
     try plugin.initialize()
     return true
   } catch {
-    // Diagnostics are emitted in the caller.
-    // FIXME: Return what happened or emit diagnostics here.
+    diagEngine?.diagnose(
+      message: "compiler plugin not loaded: '\(plugin.executableFilePath); failed to initialize",
+      severity: .warning)
     return false
   }
 }
@@ -56,12 +61,19 @@ func swift_ASTGen_pluginServerLoadLibraryPlugin(
   cxxDiagnosticEngine: UnsafeMutablePointer<UInt8>?
 ) -> Bool {
   let plugin =  CompilerPlugin(opaqueHandle: opaqueHandle)
+  let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: cxxDiagnosticEngine)
+
+  if plugin.capability?.features.contains(.loadPluginLibrary) != true {
+    // This happens only if invalid plugin server was passed to `-external-plugin-path`.
+    diagEngine?.diagnose(
+      message: "compiler plugin not loaded: '\(libraryPath); invalid plugin server",
+      severity: .warning)
+    return false
+  }
   assert(plugin.capability?.features.contains(.loadPluginLibrary) == true)
   let libraryPath = String(cString: libraryPath)
   let moduleName = String(cString: moduleName)
 
-  let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: cxxDiagnosticEngine)
-
   do {
     let result = try plugin.sendMessageAndWaitWithoutLock(
       .loadPluginLibrary(libraryPath: libraryPath, moduleName: moduleName)
@@ -72,7 +84,9 @@ func swift_ASTGen_pluginServerLoadLibraryPlugin(
     diagEngine?.emit(diagnostics);
     return loaded
   } catch {
-    diagEngine?.diagnose(error: error)
+    diagEngine?.diagnose(
+      message: "compiler plugin not loaded: '\(libraryPath); \(error)",
+      severity: .warning)
     return false
   }
 }
@@ -167,6 +181,10 @@ struct CompilerPlugin {
     }
     return nil
   }
+
+  var executableFilePath: String {
+    return String(cString: Plugin_getExecutableFilePath(opaqueHandle))
+  }
 }
 
 class PluginDiagnosticsEngine {
@@ -288,6 +306,10 @@ class PluginDiagnosticsEngine {
     )
   }
 
+  func diagnose(message: String, severity: PluginMessage.Diagnostic.Severity) {
+    self.emitSingle(message: message, severity: severity, position: .invalid)
+  }
+
   /// Produce the C++ source location for a given position based on a
   /// syntax node.
   private func cxxSourceLocation(

lib/Sema/TypeCheckMacros.cpp

@@ -311,8 +311,6 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) {
   if (!executablePlugin->isInitialized()) {
 #if SWIFT_SWIFT_PARSER
     if (!swift_ASTGen_initializePlugin(executablePlugin, &ctx.Diags)) {
-      ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded,
-                         executablePluginPath, "failed to initialize plugin");
       return nullptr;
     }
     executablePlugin->setCleanup([executablePlugin] {

test/Macros/macro_plugin_broken.swift

@@ -19,7 +19,7 @@
 
 // RUN: c-index-test -read-diagnostics %t/macro_expand.dia 2>&1 | %FileCheck -check-prefix CHECK %s
 
-// CHECK: (null):0:0: warning: compiler plugin not loaded: {{.+}}broken-plugin; loader error: failed to initialize plugin
+// CHECK: (null):0:0: warning: compiler plugin not loaded: {{.+}}broken-plugin; failed to initialize
 // CHECK: test.swift:1:33: warning: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro';
 // CHECK: test.swift:4:7: error: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro';
 // CHECK: +-{{.+}}test.swift:1:33: note: 'fooMacro' declared here

test/Macros/macro_plugin_error.swift

@@ -23,12 +23,12 @@
 
 // CHECK: ->(plugin:[[#PID1:]]) {"getCapability":{}}
 // CHECK-NEXT: <-(plugin:[[#PID1]]) {"getCapabilityResult":{"capability":{"protocolVersion":1}}}
-// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":6,"offset":200},"source":"#fooMacro(1)"}}}
+// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":6,"offset":[[#]]},"source":"#fooMacro(1)"}}}
 // CHECK-NEXT: <-(plugin:[[#PID1]]) {"invalidResponse":{}}
-// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":8,"offset":304},"source":"#fooMacro(2)"}}}
+// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":8,"offset":[[#]]},"source":"#fooMacro(2)"}}}
 // ^ This messages causes the mock plugin exit because there's no matching expected message.
 
-// CHECK: ->(plugin:[[#PID2:]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":10,"offset":386},"source":"#fooMacro(3)"}}}
+// CHECK: ->(plugin:[[#PID2:]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":10,"offset":[[#]]},"source":"#fooMacro(3)"}}}
 // CHECK-NEXT: <-(plugin:[[#PID2:]]) {"expandFreestandingMacroResult":{"diagnostics":[],"expandedSource":"3.description"}}
 
 //--- test.swift
@@ -40,7 +40,7 @@ func test() {
   let _: String = #fooMacro(1)
   // expected-error @-1 {{typeMismatch(swiftASTGen.PluginToHostMessage}}
   let _: String = #fooMacro(2)
-  // expected-error @-1 {{failedToReceiveMessage}}
+  // expected-error @-1 {{failed to receive result from plugin (from macro 'fooMacro')}}
   let _: String = #fooMacro(3)
   // ^ This should succeed.
 }

test/Macros/macro_plugin_server.swift

@@ -56,7 +56,7 @@ func testStringify(a: Int, b: Int) {
   let s1: String = #stringify(a + b).1
   print(s1)
 
-  // expected-error @+1 {{failedToReceiveMessage (from macro 'evil')}}
+  // expected-error @+1 {{failed to receive result from plugin (from macro 'evil')}}
   let s2: String = #evil(42)
   print(s2)