[Macros] Diagnose how ExternalMacroDefinition request was failed

rintaro · rintaro · commit bdd4c005e510 · 2023-10-12T16:08:48.000-07:00
Return the failure reason as the result.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -7312,7 +7312,7 @@ ERROR(macro_undefined,PointsToFirstBadToken,
       "no macro named %0", (Identifier))
 ERROR(external_macro_not_found,none,
       "external macro implementation type '%0.%1' could not be found for "
-      "macro %2", (StringRef, StringRef, DeclName))
+      "macro %2; %3", (StringRef, StringRef, DeclName, StringRef))
 ERROR(macro_must_be_defined,none,
       "macro %0 requires a definition", (DeclName))
 ERROR(external_macro_outside_macro_definition,none,
diff --git a/include/swift/AST/MacroDefinition.h b/include/swift/AST/MacroDefinition.h
@@ -18,6 +18,7 @@
 #ifndef SWIFT_AST_MACRO_DEFINITION_H
 #define SWIFT_AST_MACRO_DEFINITION_H
 
+#include "swift/Basic/StringExtras.h"
 #include "llvm/ADT/PointerUnion.h"
 
 namespace swift {
@@ -26,14 +27,25 @@ class ASTContext;
 
 /// A reference to an external macro definition that is understood by ASTGen.
 struct ExternalMacroDefinition {
-  enum class PluginKind {
+  enum class PluginKind : int8_t {
     InProcess = 0,
     Executable = 1,
+    Error = -1,
   };
   PluginKind kind;
   /// ASTGen's notion of an macro definition, which is opaque to the C++ part
-  /// of the compiler.
-  void *opaqueHandle = nullptr;
+  /// of the compiler. If 'kind' is 'PluginKind::Error', this is a C-string to
+  /// the error message
+  const void *opaqueHandle = nullptr;
+
+  static ExternalMacroDefinition error(NullTerminatedStringRef message) {
+    return ExternalMacroDefinition{PluginKind::Error,
+                                   static_cast<const void *>(message.data())};
+  }
+  bool isError() const { return kind == PluginKind::Error; }
+  llvm::StringRef getErrorMessage() const {
+    return llvm::StringRef(static_cast<const char *>(opaqueHandle));
+  }
 };
 
 /// A reference to an external macro.
diff --git a/include/swift/AST/PluginRegistry.h b/include/swift/AST/PluginRegistry.h
@@ -33,11 +33,19 @@ class LoadedLibraryPlugin {
   /// Cache of loaded symbols.
   llvm::StringMap<void *> resolvedSymbols;
 
+  /// Path to the plugin library.
+  const std::string LibraryPath;
+
 public:
-  LoadedLibraryPlugin(void *handle) : handle(handle) {}
+  LoadedLibraryPlugin(void *handle, StringRef path)
+      : handle(handle), LibraryPath(path) {}
 
   /// Finds the address of the given symbol within the library.
   void *getAddressOfSymbol(const char *symbolName);
+
+  NullTerminatedStringRef getLibraryPath() {
+    return {LibraryPath.c_str(), LibraryPath.size()};
+  }
 };
 
 /// Represent a "resolved" executable plugin.
diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h
@@ -4296,19 +4296,18 @@ class ExpandPeerMacroRequest
 /// Resolve an external macro given its module and type name.
 class ExternalMacroDefinitionRequest
     : public SimpleRequest<ExternalMacroDefinitionRequest,
-                           llvm::Optional<ExternalMacroDefinition>(
-                               ASTContext *, Identifier, Identifier),
+                           ExternalMacroDefinition(ASTContext *, Identifier,
+                                                   Identifier),
                            RequestFlags::Cached> {
 public:
   using SimpleRequest::SimpleRequest;
 
 private:
   friend SimpleRequest;
 
-  llvm::Optional<ExternalMacroDefinition> evaluate(
-      Evaluator &evaluator, ASTContext *ctx,
-      Identifier moduleName, Identifier typeName
-  ) const;
+  ExternalMacroDefinition evaluate(Evaluator &evaluator, ASTContext *ctx,
+                                   Identifier moduleName,
+                                   Identifier typeName) const;
 
 public:
   // Source location
diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def
@@ -461,7 +461,7 @@ SWIFT_REQUEST(TypeChecker, CompilerPluginLoadRequest,
               LoadedCompilerPlugin(ASTContext *, Identifier),
               Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, ExternalMacroDefinitionRequest,
-              Optional<ExternalMacroDefinition>(ASTContext *, Identifier, Identifier),
+              ExternalMacroDefinition(ASTContext *, Identifier, Identifier),
               Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, ExpandMacroExpansionDeclRequest,
               ArrayRef<Decl *>(MacroExpansionDecl *),
diff --git a/include/swift/Basic/StringExtras.h b/include/swift/Basic/StringExtras.h
@@ -21,9 +21,11 @@
 #include "swift/Basic/LLVM.h"
 #include "swift/Basic/OptionSet.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Support/Allocator.h"
 #include <iterator>
 #include <string>
@@ -492,13 +494,17 @@ class NullTerminatedStringRef {
 
   /// Create a null-terminated string, copying \p Str into \p A .
   template <typename Allocator>
-  NullTerminatedStringRef(StringRef Str, Allocator &A) : Ref("") {
-    if (Str.empty())
+  NullTerminatedStringRef(llvm::Twine Str, Allocator &A) : Ref("") {
+    if (Str.isTriviallyEmpty())
       return;
+    llvm::SmallString<0> stash;
+    auto _ref = Str.toStringRef(stash);
 
-    size_t size = Str.size();
-    char *memory = A.template Allocate<char>(size + 1);
-    memcpy(memory, Str.data(), size);
+    size_t size = _ref.size();
+    if (size == 0)
+      return;
+    char *memory = static_cast<char *>(A.Allocate(size + 1, alignof(char)));
+    memcpy(memory, _ref.data(), size);
     memory[size] = '\0';
     Ref = {memory, size};
   }
diff --git a/lib/AST/PluginRegistry.cpp b/lib/AST/PluginRegistry.cpp
@@ -67,7 +67,7 @@ PluginRegistry::loadLibraryPlugin(StringRef path) {
   }
 #endif
 
-  storage = std::make_unique<LoadedLibraryPlugin>(lib);
+  storage = std::make_unique<LoadedLibraryPlugin>(lib, path);
   return storage.get();
 }
 
diff --git a/lib/Basic/Sandbox.cpp b/lib/Basic/Sandbox.cpp
@@ -30,7 +30,7 @@ static StringRef sandboxProfile(llvm::BumpPtrAllocator &Alloc) {
   // This is required to launch any processes (execve(2)).
   contents += "(allow process-exec*)\n";
 
-  return NullTerminatedStringRef(contents, Alloc);
+  return NullTerminatedStringRef(StringRef(contents), Alloc);
 }
 #endif
 
diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp
@@ -2073,14 +2073,14 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       ExternalMacroDefinitionRequest request{
         &Ctx, external.moduleName, external.macroTypeName
       };
-      auto externalDef = evaluateOrDefault(Ctx.evaluator, request, llvm::None);
-      if (!externalDef) {
-        MD->diagnose(
-            diag::external_macro_not_found,
-            external.moduleName.str(),
-            external.macroTypeName.str(),
-            MD->getName()
-        ).limitBehavior(DiagnosticBehavior::Warning);
+      auto externalDef =
+          evaluateOrDefault(Ctx.evaluator, request,
+                            ExternalMacroDefinition::error("failed request"));
+      if (externalDef.isError()) {
+        MD->diagnose(diag::external_macro_not_found, external.moduleName.str(),
+                     external.macroTypeName.str(), MD->getName(),
+                     externalDef.getErrorMessage())
+            .limitBehavior(DiagnosticBehavior::Warning);
       }
 
       break;
diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp
@@ -65,12 +65,12 @@ extern "C" void swift_ASTGen_freeExpansionReplacements(
     ptrdiff_t numReplacements);
 
 extern "C" ptrdiff_t swift_ASTGen_expandFreestandingMacro(
-    void *diagEngine, void *macro, uint8_t externalKind,
+    void *diagEngine, const void *macro, uint8_t externalKind,
     const char *discriminator, uint8_t rawMacroRole, void *sourceFile,
     const void *sourceLocation, BridgedString *evaluatedSourceOut);
 
 extern "C" ptrdiff_t swift_ASTGen_expandAttachedMacro(
-    void *diagEngine, void *macro, uint8_t externalKind,
+    void *diagEngine, const void *macro, uint8_t externalKind,
     const char *discriminator, const char *qualifiedType,
     const char *conformances, uint8_t rawMacroRole, void *customAttrSourceFile,
     const void *customAttrSourceLocation, void *declarationSourceFile,
@@ -381,7 +381,7 @@ CompilerPluginLoadRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
   return nullptr;
 }
 
-static llvm::Optional<ExternalMacroDefinition>
+static ExternalMacroDefinition
 resolveInProcessMacro(ASTContext &ctx, Identifier moduleName,
                       Identifier typeName, LoadedLibraryPlugin *plugin) {
 #if SWIFT_BUILD_SWIFT_SYNTAX
@@ -398,13 +398,27 @@ resolveInProcessMacro(ASTContext &ctx, Identifier moduleName,
 
       return ExternalMacroDefinition{
           ExternalMacroDefinition::PluginKind::InProcess, inProcess};
+    } else {
+      NullTerminatedStringRef err(
+          "type '" + 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() +
+                                  "' could not be found in library plugin '" +
+                                  StringRef(plugin->getLibraryPath()) + "'",
+                              ctx);
+  return ExternalMacroDefinition::error(err);
 #endif
-  return llvm::None;
+  return ExternalMacroDefinition::error("macro is not supported");
 }
 
-static llvm::Optional<ExternalMacroDefinition>
+static ExternalMacroDefinition
 resolveExecutableMacro(ASTContext &ctx,
                        LoadedExecutablePlugin *executablePlugin,
                        Identifier moduleName, Identifier typeName) {
@@ -417,11 +431,17 @@ resolveExecutableMacro(ASTContext &ctx,
     return ExternalMacroDefinition{
         ExternalMacroDefinition::PluginKind::Executable, execMacro};
   }
+  NullTerminatedStringRef err(
+      "macro implementation type '" + moduleName.str() + "." + typeName.str() +
+          "' could not be found in executable plugin" +
+          StringRef(executablePlugin->getExecutablePath()),
+      ctx);
+  return ExternalMacroDefinition::error(err);
 #endif
-  return llvm::None;
+  return ExternalMacroDefinition::error("macro is not supported");
 }
 
-llvm::Optional<ExternalMacroDefinition>
+ExternalMacroDefinition
 ExternalMacroDefinitionRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
                                          Identifier moduleName,
                                          Identifier typeName) const {
@@ -432,19 +452,17 @@ ExternalMacroDefinitionRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
       evaluateOrDefault(evaluator, loadRequest, nullptr);
 
   if (auto loadedLibrary = loaded.getAsLibraryPlugin()) {
-    if (auto inProcess = resolveInProcessMacro(
-            *ctx, moduleName, typeName, loadedLibrary))
-      return *inProcess;
+    return resolveInProcessMacro(*ctx, moduleName, typeName, loadedLibrary);
   }
 
   if (auto *executablePlugin = loaded.getAsExecutablePlugin()) {
-    if (auto executableMacro = resolveExecutableMacro(*ctx, executablePlugin,
-                                                      moduleName, typeName)) {
-      return executableMacro;
-    }
+    return resolveExecutableMacro(*ctx, executablePlugin, moduleName, typeName);
   }
 
-  return llvm::None;
+  NullTerminatedStringRef err("plugin that can handle module '" +
+                                  moduleName.str() + "' not found",
+                              *ctx);
+  return ExternalMacroDefinition::error(err);
 }
 
 /// Adjust the given mangled name for a macro expansion to produce a valid
@@ -1028,11 +1046,14 @@ evaluateFreestandingMacro(FreestandingMacroExpansion *expansion,
     auto external = macroDef.getExternalMacro();
     ExternalMacroDefinitionRequest request{&ctx, external.moduleName,
                                            external.macroTypeName};
-    auto externalDef = evaluateOrDefault(ctx.evaluator, request, llvm::None);
-    if (!externalDef) {
+    auto externalDef =
+        evaluateOrDefault(ctx.evaluator, request,
+                          ExternalMacroDefinition::error("request error"));
+    if (externalDef.isError()) {
       ctx.Diags.diagnose(loc, diag::external_macro_not_found,
                          external.moduleName.str(),
-                         external.macroTypeName.str(), macro->getName());
+                         external.macroTypeName.str(), macro->getName(),
+                         externalDef.getErrorMessage());
       macro->diagnose(diag::decl_declared_here, macro);
       return nullptr;
     }
@@ -1062,9 +1083,10 @@ evaluateFreestandingMacro(FreestandingMacroExpansion *expansion,
       return nullptr;
 
     BridgedString evaluatedSourceOut{nullptr, 0};
+    assert(!externalDef.isError());
     swift_ASTGen_expandFreestandingMacro(
-        &ctx.Diags, externalDef->opaqueHandle,
-        static_cast<uint32_t>(externalDef->kind), discriminator->c_str(),
+        &ctx.Diags, externalDef.opaqueHandle,
+        static_cast<uint32_t>(externalDef.kind), discriminator->c_str(),
         getRawMacroRole(macroRole), astGenSourceFile,
         expansion->getSourceRange().Start.getOpaquePointerValue(),
         &evaluatedSourceOut);
@@ -1301,13 +1323,14 @@ static SourceFile *evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo,
     ExternalMacroDefinitionRequest request{
         &ctx, external.moduleName, external.macroTypeName
     };
-    auto externalDef = evaluateOrDefault(ctx.evaluator, request, llvm::None);
-    if (!externalDef) {
+    auto externalDef =
+        evaluateOrDefault(ctx.evaluator, request,
+                          ExternalMacroDefinition::error("failed request"));
+    if (externalDef.isError()) {
       attachedTo->diagnose(diag::external_macro_not_found,
-                        external.moduleName.str(),
-                        external.macroTypeName.str(),
-                        macro->getName()
-      );
+                           external.moduleName.str(),
+                           external.macroTypeName.str(), macro->getName(),
+                           externalDef.getErrorMessage());
       macro->diagnose(diag::decl_declared_here, macro);
       return nullptr;
     }
@@ -1339,9 +1362,10 @@ static SourceFile *evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo,
       searchDecl = var->getParentPatternBinding();
 
     BridgedString evaluatedSourceOut{nullptr, 0};
+    assert(!externalDef.isError());
     swift_ASTGen_expandAttachedMacro(
-        &ctx.Diags, externalDef->opaqueHandle,
-        static_cast<uint32_t>(externalDef->kind), discriminator->c_str(),
+        &ctx.Diags, externalDef.opaqueHandle,
+        static_cast<uint32_t>(externalDef.kind), discriminator->c_str(),
         extendedType.c_str(), conformanceList.c_str(), getRawMacroRole(role),
         astGenAttrSourceFile, attr->AtLoc.getOpaquePointerValue(),
         astGenDeclSourceFile, searchDecl->getStartLoc().getOpaquePointerValue(),
diff --git a/test/Macros/Inputs/syntax_macro_definitions.swift b/test/Macros/Inputs/syntax_macro_definitions.swift
@@ -2071,3 +2071,12 @@ public struct FakeCodeItemMacro: DeclarationMacro, PeerMacro {
     return ["if true { return }"]
   }
 }
+
+public struct NotMacroStruct {
+  public static func expansion(
+    of macro: some FreestandingMacroExpansionSyntax,
+    in context: some MacroExpansionContext
+  ) -> ExprSyntax {
+    fatalError()
+  }
+}
diff --git a/test/Macros/macro_expand.swift b/test/Macros/macro_expand.swift
@@ -612,3 +612,12 @@ macro DefineComparableType() = #externalMacro(module: "MacroDefinition", type: "
 struct HasNestedType {
   #DefineComparableType
 }
+
+#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 '}}
+@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 '}}
+#endif

Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@ PluginRegistry::loadLibraryPlugin(StringRef path) {`
`67`	`67`	`}`
`68`	`68`	`#endif`
`69`	`69`
`70`		`- storage = std::make_unique<LoadedLibraryPlugin>(lib);`
	`70`	`+ storage = std::make_unique<LoadedLibraryPlugin>(lib, path);`
`71`	`71`	`return storage.get();`
`72`	`72`	`}`
`73`	`73`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ static StringRef sandboxProfile(llvm::BumpPtrAllocator &Alloc) {`
`30`	`30`	`// This is required to launch any processes (execve(2)).`
`31`	`31`	`contents += "(allow process-exec*)\n";`
`32`	`32`
`33`		`- return NullTerminatedStringRef(contents, Alloc);`
	`33`	`+ return NullTerminatedStringRef(StringRef(contents), Alloc);`
`34`	`34`	`}`
`35`	`35`	`#endif`
`36`	`36`