Merge pull request #65630 from rintaro/macro-plugin-intiialize-rdar108022847

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/AST/PluginRegistry.cpp

@@ -178,7 +178,8 @@ LoadedExecutablePlugin::PluginProcess::~PluginProcess() {
 
 LoadedExecutablePlugin::~LoadedExecutablePlugin() {
   // Let ASTGen to cleanup things.
-  this->cleanup();
+  if (this->cleanup)
+    this->cleanup();
 }
 
 ssize_t LoadedExecutablePlugin::PluginProcess::read(void *buf,

lib/ASTGen/Sources/ASTGen/PluginHost.swift

@@ -15,19 +15,32 @@ 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")
 public func _initializePlugin(
-  opaqueHandle: UnsafeMutableRawPointer
-) {
+  opaqueHandle: UnsafeMutableRawPointer,
+  cxxDiagnosticEngine: UnsafeMutablePointer<UInt8>?
+) -> Bool {
   let plugin = CompilerPlugin(opaqueHandle: opaqueHandle)
-  plugin.initialize()
+  let diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: cxxDiagnosticEngine)
+
+  do {
+    try plugin.initialize()
+    return true
+  } catch {
+    diagEngine?.diagnose(
+      message: "compiler plugin not loaded: '\(plugin.executableFilePath); failed to initialize",
+      severity: .warning)
+    return false
+  }
 }
 
 @_cdecl("swift_ASTGen_deinitializePlugin")
@@ -48,17 +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?
-  if let cxxDiagnosticEngine = cxxDiagnosticEngine {
-    diagEngine = PluginDiagnosticsEngine(cxxDiagnosticEngine: cxxDiagnosticEngine)
-  } else {
-    diagEngine = nil
-  }
-
   do {
     let result = try plugin.sendMessageAndWaitWithoutLock(
       .loadPluginLibrary(libraryPath: libraryPath, moduleName: moduleName)
@@ -69,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
   }
 }
@@ -136,20 +153,15 @@ struct CompilerPlugin {
   }
 
   /// Initialize the plugin. This should be called inside lock.
-  func initialize() {
-    do {
-      // Get capability.
-      let response = try self.sendMessageAndWaitWithoutLock(.getCapability)
-      guard case .getCapabilityResult(let capability) = response else {
-        throw PluginError.invalidReponseKind
-      }
-      let ptr = UnsafeMutablePointer<Capability>.allocate(capacity: 1)
-      ptr.initialize(to: .init(capability))
-      Plugin_setCapability(opaqueHandle, UnsafeRawPointer(ptr))
-    } catch {
-      assertionFailure(String(describing: error))
-      return
+  func initialize() throws {
+    // Get capability.
+    let response = try self.sendMessageAndWaitWithoutLock(.getCapability)
+    guard case .getCapabilityResult(let capability) = response else {
+      throw PluginError.invalidReponseKind
     }
+    let ptr = UnsafeMutablePointer<Capability>.allocate(capacity: 1)
+    ptr.initialize(to: .init(capability))
+    Plugin_setCapability(opaqueHandle, UnsafeRawPointer(ptr))
   }
 
   func deinitialize() {
@@ -169,6 +181,10 @@ struct CompilerPlugin {
     }
     return nil
   }
+
+  var executableFilePath: String {
+    return String(cString: Plugin_getExecutableFilePath(opaqueHandle))
+  }
 }
 
 class PluginDiagnosticsEngine {
@@ -179,6 +195,14 @@ class PluginDiagnosticsEngine {
     self.cxxDiagnosticEngine = cxxDiagnosticEngine
   }
 
+  /// Failable convenience initializer for optional cxx engine pointer.
+  convenience init?(cxxDiagnosticEngine: UnsafeMutablePointer<UInt8>?) {
+    guard let cxxDiagnosticEngine = cxxDiagnosticEngine else {
+      return nil
+    }
+    self.init(cxxDiagnosticEngine: cxxDiagnosticEngine)
+  }
+
   /// Register an 'ExportedSourceFile' to the engine. So the engine can get
   /// C++ SourceLoc from a pair of filename and offset.
   func add(exportedSourceFile: UnsafePointer<ExportedSourceFile>) {
@@ -282,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

@@ -21,6 +21,7 @@
 #include "swift/AST/ASTMangler.h"
 #include "swift/AST/ASTNode.h"
 #include "swift/AST/CASTBridging.h"
+#include "swift/AST/DiagnosticsFrontend.h"
 #include "swift/AST/Expr.h"
 #include "../AST/InlinableText.h"
 #include "swift/AST/MacroDefinition.h"
@@ -75,7 +76,7 @@ extern "C" ptrdiff_t swift_ASTGen_expandAttachedMacro(
     void *parentDeclSourceFile, const void *parentDeclSourceLocation,
     const char **evaluatedSource, ptrdiff_t *evaluatedSourceLength);
 
-extern "C" void swift_ASTGen_initializePlugin(void *handle);
+extern "C" bool swift_ASTGen_initializePlugin(void *handle, void *diagEngine);
 extern "C" void swift_ASTGen_deinitializePlugin(void *handle);
 extern "C" bool swift_ASTGen_pluginServerLoadLibraryPlugin(
     void *handle, const char *libraryPath, const char *moduleName,
@@ -316,7 +317,9 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) {
   // But plugin loading is in libAST and it can't link ASTGen symbols.
   if (!executablePlugin->isInitialized()) {
 #if SWIFT_SWIFT_PARSER
-    swift_ASTGen_initializePlugin(executablePlugin);
+    if (!swift_ASTGen_initializePlugin(executablePlugin, &ctx.Diags)) {
+      return nullptr;
+    }
     executablePlugin->setCleanup([executablePlugin] {
       swift_ASTGen_deinitializePlugin(executablePlugin);
     });
@@ -328,7 +331,9 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) {
 #if SWIFT_SWIFT_PARSER
     llvm::SmallString<128> resolvedLibraryPath;
     auto fs = ctx.SourceMgr.getFileSystem();
-    if (fs->getRealPath(libraryPath, resolvedLibraryPath)) {
+    if (auto err = fs->getRealPath(libraryPath, resolvedLibraryPath)) {
+      ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded,
+                         executablePluginPath, err.message());
       return nullptr;
     }
     std::string resolvedLibraryPathStr(resolvedLibraryPath);

test/Macros/macro_plugin_broken.swift

@@ -0,0 +1,40 @@
+// REQUIRES: swift_swift_parser
+
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %host-build-swift \
+// RUN:   -swift-version 5 -o %t/broken-plugin \
+// RUN:   -module-name=TestPlugin \
+// RUN:   %t/broken_plugin.swift \
+// RUN:   -g -no-toolchain-stdlib-rpath -swift-version 5
+
+// RUN: not %swift-target-frontend \
+// RUN:   -typecheck \
+// RUN:   -swift-version 5 -enable-experimental-feature Macros \
+// RUN:   -load-plugin-executable %t/broken-plugin#TestPlugin \
+// RUN:   -module-name MyApp \
+// RUN:   -serialize-diagnostics-path %t/macro_expand.dia \
+// RUN:   %t/test.swift
+
+// 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; 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.swift
+@freestanding(expression) macro fooMacro(_: Any) -> String = #externalMacro(module: "TestPlugin", type: "FooMacro")
+
+func test() {
+  _ = #fooMacro(1)
+}
+
+//--- broken_plugin.swift
+func minusTen(value: UInt) -> UInt {
+  // Causes crash.
+  return value - 10
+}
+
+_ = minusTen(value: 5)

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)