[AST] PluginRegistry returns Error/Expected instead of errorMsg out

Author: rintaro

include/swift/AST/PluginRegistry.h

@@ -14,6 +14,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/Program.h"
 
 #include <mutex>
@@ -52,10 +53,10 @@ class LoadedExecutablePlugin {
   void unlock() { mtx.unlock(); }
 
   /// Send a message to the plugin.
-  bool sendMessage(llvm::StringRef message) const;
+  llvm::Error sendMessage(llvm::StringRef message) const;
 
   /// Wait for a message from plugin and returns it.
-  std::string waitForNextMessage() const;
+  llvm::Expected<std::string> waitForNextMessage() const;
 
   bool isInitialized() const { return bool(cleanup); }
   void setCleanup(std::function<void(void)> cleanup) {
@@ -77,9 +78,9 @@ class PluginRegistry {
       LoadedPluginExecutables;
 
 public:
-  bool loadLibraryPlugin(llvm::StringRef path, const char *&errorMsg);
-  LoadedExecutablePlugin *loadExecutablePlugin(llvm::StringRef path,
-                                               const char *&errorMsg);
+  llvm::Error loadLibraryPlugin(llvm::StringRef path);
+  llvm::Expected<LoadedExecutablePlugin *>
+  loadExecutablePlugin(llvm::StringRef path);
 
   const llvm::StringMap<void *> &getLoadedLibraryPlugins() const {
     return LoadedPluginLibraries;

lib/AST/ASTContext.cpp

@@ -6227,7 +6227,6 @@ PluginRegistry *ASTContext::getPluginRegistry() const {
 }
 
 void ASTContext::loadCompilerPlugins() {
-  const char *errorMsg = nullptr;
   auto fs = this->SourceMgr.getFileSystem();
   for (auto &path : SearchPathOpts.getCompilerPluginLibraryPaths()) {
     SmallString<128> resolvedPath;
@@ -6236,9 +6235,9 @@ void ASTContext::loadCompilerPlugins() {
                      err.message());
       continue;
     }
-    if (getPluginRegistry()->loadLibraryPlugin(resolvedPath, errorMsg)) {
+    if (auto error = getPluginRegistry()->loadLibraryPlugin(resolvedPath)) {
       Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded, path,
-                     errorMsg);
+                     llvm::toString(std::move(error)));
     }
   }
 
@@ -6339,12 +6338,11 @@ ASTContext::lookupExecutablePluginByModuleName(Identifier moduleName) {
   }
 
   // Load the plugin.
-  const char *errorMsg;
-  auto plugin = getPluginRegistry()->loadExecutablePlugin(resolvedPath, errorMsg);
+  auto plugin = getPluginRegistry()->loadExecutablePlugin(resolvedPath);
   if (!plugin) {
     Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded, path,
-                   errorMsg);
+                   llvm::toString(plugin.takeError()));
   }
 
-  return plugin;
+  return plugin.get();
 }

lib/AST/CASTBridging.cpp

@@ -651,14 +651,19 @@ void Plugin_unlock(PluginHandle handle) {
 bool Plugin_sendMessage(PluginHandle handle, const BridgedData data) {
   auto *plugin = static_cast<LoadedExecutablePlugin *>(handle);
   StringRef message(data.baseAddress, data.size);
-  return plugin->sendMessage(message);
+  return bool(plugin->sendMessage(message));
 }
 
 bool Plugin_waitForNextMessage(PluginHandle handle, BridgedData *out) {
   auto *plugin = static_cast<LoadedExecutablePlugin *>(handle);
   auto result = plugin->waitForNextMessage();
-  auto outPtr = malloc(result.size());
-  memcpy(outPtr, result.data(), result.size());
-  *out = BridgedData{(const char *)outPtr, result.size()};
+  if (!result) {
+    return true;
+  }
+  auto &message = result.get();
+  auto size = message.size();
+  auto outPtr = malloc(size);
+  memcpy(outPtr, message.data(), size);
+  *out = BridgedData{(const char *)outPtr, size};
   return false;
 }

lib/AST/PluginRegistry.cpp

@@ -41,35 +41,34 @@ extern "C" void swift_ASTGen_destroyCompilerPluginCapability(void *value);
 
 using namespace swift;
 
-bool PluginRegistry::loadLibraryPlugin(StringRef path, const char *&errorMsg) {
+llvm::Error PluginRegistry::loadLibraryPlugin(StringRef path) {
   if (LoadedPluginLibraries.find(path) != LoadedPluginLibraries.end()) {
     // Already loaded.
-    return false;
+    return llvm::Error::success();
   }
-  errorMsg = nullptr;
   void *lib = nullptr;
 #if defined(_WIN32)
   lib = LoadLibraryA(path.str().c_str());
   if (!lib) {
-    errorMsg = "faild";
+    return llvm::createStringError(std::errc::not_supported, "failed");
     return true;
   }
 #else
   lib = dlopen(path.str().c_str(), RTLD_LAZY | RTLD_LOCAL);
   if (!lib) {
-    errorMsg = "Unsupported platform";
-    return true;
+    return llvm::createStringError(std::errc::not_supported,
+                                   "unsupported platform");
   }
 #endif
   LoadedPluginLibraries.insert({path, lib});
-  return false;
+  return llvm::Error::success();
 }
 
-LoadedExecutablePlugin *
-PluginRegistry::loadExecutablePlugin(StringRef path, const char *&errorMsg) {
+llvm::Expected<LoadedExecutablePlugin *>
+PluginRegistry::loadExecutablePlugin(StringRef path) {
   llvm::sys::fs::file_status stat;
   if (auto err = llvm::sys::fs::status(path, stat)) {
-    errorMsg = err.message().c_str();
+    return llvm::errorCodeToError(err);
   }
 
   // See if the plugin is already loaded.
@@ -84,13 +83,13 @@ PluginRegistry::loadExecutablePlugin(StringRef path, const char *&errorMsg) {
   }
 
   if (!llvm::sys::fs::exists(stat)) {
-    errorMsg = "not found";
-    return nullptr;
+    return llvm::createStringError(std::errc::no_such_file_or_directory,
+                                   "not found");
   }
 
   if (!llvm::sys::fs::can_execute(path)) {
-    errorMsg = "not executable";
-    return nullptr;
+    return llvm::createStringError(std::errc::permission_denied,
+                                   "not executable");
   }
 
   // Create command line arguments.
@@ -103,8 +102,7 @@ PluginRegistry::loadExecutablePlugin(StringRef path, const char *&errorMsg) {
   // Launch.
   auto childInfo = ExecuteWithPipe(command[0], command);
   if (!childInfo) {
-    errorMsg = "Failed to execute";
-    return nullptr;
+    return llvm::errorCodeToError(childInfo.getError());
   }
 
   plugin = std::unique_ptr<LoadedExecutablePlugin>(new LoadedExecutablePlugin(
@@ -163,7 +161,7 @@ ssize_t LoadedExecutablePlugin::write(const void *buf, size_t nbyte) const {
   return nbyte - bytesToWrite;
 }
 
-bool LoadedExecutablePlugin::sendMessage(llvm::StringRef message) const {
+llvm::Error LoadedExecutablePlugin::sendMessage(llvm::StringRef message) const {
   ssize_t writtenSize = 0;
 
   const char *data = message.data();
@@ -180,10 +178,10 @@ bool LoadedExecutablePlugin::sendMessage(llvm::StringRef message) const {
   writtenSize = write(data, size);
   assert(writtenSize == ssize_t(size) && "failed to write plugin message data");
 
-  return false;
+  return llvm::Error::success();
 }
 
-std::string LoadedExecutablePlugin::waitForNextMessage() const {
+llvm::Expected<std::string> LoadedExecutablePlugin::waitForNextMessage() const {
   ssize_t readSize = 0;
 
   // Read header (message size).