[Macros] Tweak macro resolution error messages

Author: rintaro

lib/ASTGen/Sources/ASTGen/PluginHost.swift

@@ -58,16 +58,12 @@ func swift_ASTGen_pluginServerLoadLibraryPlugin(
   opaqueHandle: UnsafeMutableRawPointer,
   libraryPath: UnsafePointer<CChar>,
   moduleName: UnsafePointer<CChar>,
-  cxxDiagnosticEngine: UnsafeMutableRawPointer?
+  errorOut: UnsafeMutablePointer<BridgedString>?
 ) -> 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)
+    errorOut?.pointee = allocateBridgedString("compiler plugin not loaded: '\(libraryPath); invalid plugin server")
     return false
   }
   assert(plugin.capability?.features.contains(.loadPluginLibrary) == true)
@@ -81,12 +77,15 @@ func swift_ASTGen_pluginServerLoadLibraryPlugin(
     guard case .loadPluginLibraryResult(let loaded, let diagnostics) = result else {
       throw PluginError.invalidReponseKind
     }
-    diagEngine?.emit(diagnostics);
-    return loaded
+    if loaded {
+      assert(diagnostics.isEmpty)
+      return true
+    }
+    var errorMsgs = diagnostics.map({$0.message}).joined(separator: ", ");
+    errorOut?.pointee = allocateBridgedString(errorMsgs);
+    return false
   } catch {
-    diagEngine?.diagnose(
-      message: "compiler plugin not loaded: '\(libraryPath); \(error)",
-      severity: .warning)
+    errorOut?.pointee = allocateBridgedString("\(error)")
     return false
   }
 }

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -2075,7 +2075,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       };
       auto externalDef =
           evaluateOrDefault(Ctx.evaluator, request,
-                            ExternalMacroDefinition::error("failed request"));
+                            ExternalMacroDefinition::error("unknown error"));
       if (externalDef.isError()) {
         MD->diagnose(diag::external_macro_not_found, external.moduleName.str(),
                      external.macroTypeName.str(), MD->getName(),

lib/Sema/TypeCheckMacros.cpp

@@ -81,7 +81,7 @@ 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,
-    void *diagEngine);
+    BridgedString *errorOut);
 
 static inline StringRef toStringRef(BridgedString bridged) {
   return {reinterpret_cast<const char *>(bridged.data), size_t(bridged.length)};
@@ -292,9 +292,8 @@ initializeExecutablePlugin(ASTContext &ctx,
 #if SWIFT_BUILD_SWIFT_SYNTAX
     if (!swift_ASTGen_initializePlugin(executablePlugin, &ctx.Diags)) {
       return llvm::createStringError(
-          llvm::inconvertibleErrorCode(),
-          "failed to initialize executable plugin '" +
-              StringRef(executablePlugin->getExecutablePath()) + "'");
+          llvm::inconvertibleErrorCode(), "'%s' produced malformed response",
+          executablePlugin->getExecutablePath().data());
     }
 
     // Resend the compiler capability on reconnect.
@@ -322,23 +321,28 @@ initializeExecutablePlugin(ASTContext &ctx,
     std::string resolvedLibraryPathStr(resolvedLibraryPath);
     std::string moduleNameStr(moduleName.str());
 
+    BridgedString errorOut{nullptr, 0};
     bool loaded = swift_ASTGen_pluginServerLoadLibraryPlugin(
         executablePlugin, resolvedLibraryPathStr.c_str(), moduleNameStr.c_str(),
-        &ctx.Diags);
-    if (!loaded)
+        &errorOut);
+    if (!loaded) {
+      SWIFT_DEFER { swift_ASTGen_freeBridgedString(errorOut); };
       return llvm::createStringError(
           llvm::inconvertibleErrorCode(),
-          "failed to load library plugin '" + resolvedLibraryPathStr +
-              "' in plugin server '" +
-              StringRef(executablePlugin->getExecutablePath()) + "'");
+          "failed to load library plugin '%s' in plugin server '%s'; %s",
+          resolvedLibraryPathStr.c_str(),
+          executablePlugin->getExecutablePath().data(), errorOut.data);
+    }
+
+    assert(errorOut.data == nullptr);
 
     // Set a callback to load the library again on reconnections.
     auto *callback = new std::function<void(void)>(
         [executablePlugin, resolvedLibraryPathStr, moduleNameStr]() {
           (void)swift_ASTGen_pluginServerLoadLibraryPlugin(
               executablePlugin, resolvedLibraryPathStr.c_str(),
               moduleNameStr.c_str(),
-              /*diags=*/nullptr);
+              /*errorOut=*/nullptr);
         });
     executablePlugin->addOnReconnect(callback);
 
@@ -359,7 +363,7 @@ CompilerPluginLoadRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
   PluginLoader &loader = ctx->getPluginLoader();
   const auto &entry = loader.lookupPluginByModuleName(moduleName);
 
-  std::string errorMessage;
+  SmallString<0> errorMessage;
 
   if (!entry.executablePath.empty()) {
     llvm::Expected<LoadedExecutablePlugin *> executablePlugin =
@@ -376,9 +380,12 @@ CompilerPluginLoadRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
     }
     if (executablePlugin)
       return executablePlugin.get();
-    llvm::handleAllErrors(
-        executablePlugin.takeError(),
-        [&](const llvm::ErrorInfoBase &err) { errorMessage += err.message(); });
+    llvm::handleAllErrors(executablePlugin.takeError(),
+                          [&](const llvm::ErrorInfoBase &err) {
+                            if (!errorMessage.empty())
+                              errorMessage += ", ";
+                            errorMessage += err.message();
+                          });
   } else if (!entry.libraryPath.empty()) {
 
     llvm::Expected<LoadedLibraryPlugin *> libraryPlugin =
@@ -393,6 +400,8 @@ CompilerPluginLoadRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
     } else {
       llvm::handleAllErrors(libraryPlugin.takeError(),
                             [&](const llvm::ErrorInfoBase &err) {
+                              if (!errorMessage.empty())
+                                errorMessage += ", ";
                               errorMessage += err.message();
                             });
     }
@@ -401,9 +410,8 @@ CompilerPluginLoadRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
     NullTerminatedStringRef err(errorMessage, *ctx);
     return CompilerPluginLoadResult::error(err);
   } else {
-    NullTerminatedStringRef errMsg("plugin that can handle module '" +
-                                       moduleName.str() + "' not found",
-                                   *ctx);
+    NullTerminatedStringRef errMsg(
+        "plugin for module '" + moduleName.str() + "' not found", *ctx);
     return CompilerPluginLoadResult::error(errMsg);
   }
 }
@@ -427,22 +435,22 @@ resolveInProcessMacro(ASTContext &ctx, Identifier moduleName,
           ExternalMacroDefinition::PluginKind::InProcess, inProcess};
     } else {
       NullTerminatedStringRef err(
-          "type '" + moduleName.str() + "." + typeName.str() +
+          "'" + moduleName.str() + "." + typeName.str() +
               "' is not a valid macro implementation type in library plugin '" +
               StringRef(plugin->getLibraryPath()) + "'",
           ctx);
 
       return ExternalMacroDefinition::error(err);
     }
   }
-  NullTerminatedStringRef err("macro implementation type '" + moduleName.str() +
-                                  "." + typeName.str() +
+  NullTerminatedStringRef err("'" + moduleName.str() + "." + typeName.str() +
                                   "' could not be found in library plugin '" +
                                   StringRef(plugin->getLibraryPath()) + "'",
                               ctx);
   return ExternalMacroDefinition::error(err);
 #endif
-  return ExternalMacroDefinition::error("macro is not supported");
+  return ExternalMacroDefinition::error(
+      "the current compiler was not built with macro support");
 }
 
 static ExternalMacroDefinition
@@ -461,13 +469,14 @@ resolveExecutableMacro(ASTContext &ctx,
   // NOTE: this is not reachable because executable macro resolution always
   // succeeds.
   NullTerminatedStringRef err(
-      "macro implementation type '" + moduleName.str() + "." + typeName.str() +
+      "'" + moduleName.str() + "." + typeName.str() +
           "' could not be found in executable plugin" +
           StringRef(executablePlugin->getExecutablePath()),
       ctx);
   return ExternalMacroDefinition::error(err);
 #endif
-  return ExternalMacroDefinition::error("macro is not supported");
+  return ExternalMacroDefinition::error(
+      "the current compiler was not built with macro support");
 }
 
 ExternalMacroDefinition

test/Macros/macro_expand.swift

@@ -616,8 +616,8 @@ struct HasNestedType {
 #if TEST_DIAGNOSTICS
 @freestanding(expression)
 macro missingMacro() = #externalMacro(module: "MacroDefinition", type: "BluhBlah")
-// expected-warning@-1 {{external macro implementation type 'MacroDefinition.BluhBlah' could not be found for macro 'missingMacro()'; macro implementation type 'MacroDefinition.BluhBlah' could not be found in library plugin '}}
+// expected-warning@-1 {{external macro implementation type 'MacroDefinition.BluhBlah' could not be found for macro 'missingMacro()'; 'MacroDefinition.BluhBlah' could not be found in library plugin '}}
 @freestanding(expression)
 macro notMacro() = #externalMacro(module: "MacroDefinition", type: "NotMacroStruct")
-// expected-warning@-1 {{macro implementation type 'MacroDefinition.NotMacroStruct' could not be found for macro 'notMacro()'; type 'MacroDefinition.NotMacroStruct' is not a valid macro implementation type in library plugin '}}
+// expected-warning@-1 {{macro implementation type 'MacroDefinition.NotMacroStruct' could not be found for macro 'notMacro()'; 'MacroDefinition.NotMacroStruct' is not a valid macro implementation type in library plugin '}}
 #endif

test/Macros/macro_plugin_broken.swift

@@ -18,8 +18,8 @@
 
 // RUN: c-index-test -read-diagnostics %t/macro_expand.dia 2>&1 | %FileCheck -check-prefix CHECK %s
 
-// CHECK: test.swift:1:33: warning: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; failed to initialize executable plugin '{{.*}}broken-plugin'
-// CHECK: test.swift:4:7: error: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; failed to initialize executable plugin '{{.*}}broken-plugin'
+// CHECK: test.swift:1:33: warning: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; '{{.*}}broken-plugin' produced malformed response
+// CHECK: test.swift:4:7: error: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; '{{.*}}broken-plugin' produced malformed response
 // CHECK: +-{{.+}}test.swift:1:33: note: 'fooMacro' declared here
 
 //--- test.swift

test/Macros/macro_plugin_broken_shlib.swift

@@ -18,9 +18,8 @@
 // RUN: c-index-test -read-diagnostics %t/macro_expand.dia 2>&1 | %FileCheck -check-prefix SERVER %s
 
 // SERVER-NOT: {{error|warning}}
-// SERVER:  (null):0:0: error: loader error: dlopen(BUILD_DIR/{{.*}}/libTestPlugin.dylib, 0x0009): tried: 'BUILD_DIR/{{.*}}/plugins/libTestPlugin.dylib'
-// SERVER: test.swift:1:33: warning: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; failed to load library plugin 'BUILD_DIR/{{.*}}/libTestPlugin.dylib' in plugin server 'BUILD_DIR/{{.*}}/swift-plugin-server'
-// SERVER: test.swift:4:7: error: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; failed to load library plugin 'BUILD_DIR/{{.*}}/libTestPlugin.dylib' in plugin server 'BUILD_DIR/{{.*}}/swift-plugin-server'
+// SERVER: test.swift:1:33: warning: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; failed to load library plugin 'BUILD_DIR/{{.*}}/libTestPlugin.dylib' in plugin server 'BUILD_DIR/{{.*}}/swift-plugin-server'; loader error: dlopen(BUILD_DIR/{{.*}}/libTestPlugin.dylib, {{.*}}): tried: 'BUILD_DIR/{{.*}}/plugins/libTestPlugin.dylib'
+// SERVER: test.swift:4:7: error: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; failed to load library plugin 'BUILD_DIR/{{.*}}/libTestPlugin.dylib' in plugin server 'BUILD_DIR/{{.*}}/swift-plugin-server'; loader error: dlopen(BUILD_DIR/{{.*}}/libTestPlugin.dylib, {{.*}}): tried: 'BUILD_DIR/{{.*}}/plugins/libTestPlugin.dylib'
 // SERVER: test.swift:1:33: note: 'fooMacro' declared here
 // SERVER-NOT: {{error|warning}}
 

test/Macros/macros_diagnostics.swift

@@ -132,11 +132,11 @@ func testDiags(a: Int, b: Int) {
 
 func shadow(a: Int, b: Int, stringify: Int) {
   _ = #stringify(a + b)
-  // expected-error@-1{{external macro implementation type 'MacroDefinition.StringifyMacro' could not be found for macro 'stringify'; plugin that can handle module 'MacroDefinition' not found}}
+  // expected-error@-1{{external macro implementation type 'MacroDefinition.StringifyMacro' could not be found for macro 'stringify'; plugin for module 'MacroDefinition' not found}}
 }
 
 func testMissing() {
-  #missingMacro1("hello") // expected-error{{external macro implementation type 'MissingModule.MissingType' could not be found for macro 'missingMacro1'; plugin that can handle module 'MissingModule' not found}}
+  #missingMacro1("hello") // expected-error{{external macro implementation type 'MissingModule.MissingType' could not be found for macro 'missingMacro1'; plugin for module 'MissingModule' not found}}
 }
 
 @freestanding(expression) macro undefined() // expected-error{{macro 'undefined()' requires a definition}}
@@ -148,12 +148,12 @@ func testExternalMacroOutOfPlace() {
 
 @freestanding(expression)
 public macro macroWithDefaults(_: Int = 17) = #externalMacro(module: "A", type: "B")
-// expected-warning@-1{{external macro implementation type 'A.B' could not be found for macro 'macroWithDefaults'; plugin that can handle module 'A' not found}}
+// expected-warning@-1{{external macro implementation type 'A.B' could not be found for macro 'macroWithDefaults'; plugin for module 'A' not found}}
 // expected-note@-2{{'macroWithDefaults' declared here}}
 
 func callMacroWithDefaults() {
   _ = #macroWithDefaults()
-  // expected-error@-1 {{external macro implementation type 'A.B' could not be found for macro 'macroWithDefaults'; plugin that can handle module 'A' not found}}
+  // expected-error@-1 {{external macro implementation type 'A.B' could not be found for macro 'macroWithDefaults'; plugin for module 'A' not found}}
 }
 
 // Make sure we don't allow macros to prevent type folding.
@@ -164,7 +164,7 @@ public macro MacroOrType() = #externalMacro(module: "A", type: "MacroOrType")
 @freestanding(codeItem, names: named(foo))
 public macro badCodeItemMacro() = #externalMacro(module: "A", type: "B")
 // expected-error@-2{{'codeItem' macros are not allowed to introduce names}}
-// expected-warning@-2{{external macro implementation type 'A.B' could not be found for macro 'badCodeItemMacro()'; plugin that can handle module 'A' not found}}
+// expected-warning@-2{{external macro implementation type 'A.B' could not be found for macro 'badCodeItemMacro()'; plugin for module 'A' not found}}
 
 struct MacroOrType {
   typealias Nested = Int