[Macros] Small changes for review

Author: rintaro

include/swift/Basic/Program.h

@@ -58,7 +58,7 @@ struct ChildProcessInfo {
 /// \param program Path of the program to be executed
 /// \param args An array of strings that are passed to the program. The first
 /// element should be the name of the program.
-/// \param env An optional array of strings to use for the program's
+/// \param env An optional array of 'key=value 'strings to use for the program's
 /// environment.
 llvm::ErrorOr<swift::ChildProcessInfo> ExecuteWithPipe(
     llvm::StringRef program, llvm::ArrayRef<llvm::StringRef> args,

lib/AST/ASTContext.cpp

@@ -531,7 +531,7 @@ struct ASTContext::Implementation {
   llvm::StringMap<void *> LoadedSymbols;
 
   /// Map a module name to an executable plugin path that provides the module.
-  llvm::DenseMap<Identifier, std::string> ExecutablePluginPaths;
+  llvm::DenseMap<Identifier, StringRef> ExecutablePluginPaths;
 
   /// The permanent arena.
   Arena Permanent;
@@ -6256,9 +6256,9 @@ void ASTContext::loadCompilerPlugins() {
       // TODO: Error messsage.
       Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded, arg, "");
     }
-    auto pathstr = std::string(path);
+    auto pathStr = AllocateCopy(path);
     for (auto moduleName : modules) {
-      getImpl().ExecutablePluginPaths[getIdentifier(moduleName)] = pathstr;
+      getImpl().ExecutablePluginPaths[getIdentifier(moduleName)] = pathStr;
     }
   }
 }
@@ -6327,7 +6327,7 @@ ASTContext::lookupExecutablePluginByModuleName(Identifier moduleName) {
   if (found == execPluginPaths.end())
     return nullptr;
 
-  // Resolve the realpath.
+  // Let the VFS to map the path.
   auto &path = found->second;
   SmallString<128> resolvedPath;
   auto fs = this->SourceMgr.getFileSystem();

lib/ASTGen/Sources/ASTGen/Macros.swift

@@ -109,13 +109,13 @@ public func resolveExecutableMacro(
   moduleNameLength: Int,
   typeName: UnsafePointer<UInt8>,
   typeNameLength: Int,
-  pluginOpaqueHanle: UnsafeMutableRawPointer
+  pluginOpaqueHandle: UnsafeMutableRawPointer
 ) -> UnsafeRawPointer {
   let exportedPtr = UnsafeMutablePointer<ExportedExecutableMacro>.allocate(capacity: 1)
   exportedPtr.initialize(to: .init(
     moduleName: String(bufferStart: moduleName, count: moduleNameLength),
     typeName: String(bufferStart: typeName, count: typeNameLength),
-    plugin: CompilerPlugin(opaqueHandle: pluginOpaqueHanle)))
+    plugin: CompilerPlugin(opaqueHandle: pluginOpaqueHandle)))
   return UnsafeRawPointer(exportedPtr)
 }
 

lib/Basic/Program.cpp

@@ -59,7 +59,6 @@ toNullTerminatedCStringArray(ArrayRef<StringRef> array,
                              llvm::BumpPtrAllocator &Alloc) {
   size_t size = array.size();
   const char **result = Alloc.Allocate<const char *>(size + 1);
-  const char **ptr = result;
   for (size_t i = 0; i < size; ++i) {
     result[i] = NullTerminatedStringRef(array[i], Alloc).data();
   }
@@ -155,8 +154,7 @@ swift::ExecuteWithPipe(llvm::StringRef program,
     dup2(p2.write, STDOUT_FILENO);
 
     // Execute the program.
-    if (env.has_value()) {
-      const char **envp = toNullTerminatedCStringArray(*env, Alloc);
+    if (envp) {
       execve(progCStr, const_cast<char **>(argv), const_cast<char **>(envp));
     } else {
       execv(progCStr, const_cast<char **>(argv));

lib/Sema/TypeCheckMacros.cpp

@@ -356,7 +356,7 @@ resolveInProcessMacro(
 }
 
 static Optional<ExternalMacroDefinition>
-resolvExecutableMacro(ASTContext &ctx, Identifier moduleName,
+resolveExecutableMacro(ASTContext &ctx, Identifier moduleName,
                       Identifier typeName) {
 #if SWIFT_SWIFT_PARSER
   // Find macros in exectuable plugins.
@@ -374,7 +374,7 @@ resolvExecutableMacro(ASTContext &ctx, Identifier moduleName,
     });
   }
 
-  if (auto execMacro = swift_ASTGen_resolveExecutableMacro(
+  if (auto *execMacro = swift_ASTGen_resolveExecutableMacro(
           moduleName.str().data(), moduleName.str().size(),
           typeName.str().data(), typeName.str().size(), executablePlugin)) {
     // Make sure we clean up after the macro.
@@ -408,7 +408,7 @@ ExternalMacroDefinitionRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
 
   // Try executable plugins.
   if (auto executableMacro =
-          resolvExecutableMacro(*ctx, moduleName, typeName)) {
+          resolveExecutableMacro(*ctx, moduleName, typeName)) {
     return executableMacro;
   }
 

utils/mock_plugin.py

@@ -23,9 +23,9 @@
 #
 # where 'TEST_SPEC' is a list of pairs of an expected request and the result to
 # respond. For example:
-#   {'expected': {'testRequest': {'params': []}},
+#   {'expect': {'testRequest': {'params': []}},
 #    'response': {'testRequestResult': [1,2,"foo"]}}
-# this spec matches a request that is an object with 'testRequst' key with an
+# this spec matches a request that is an object with 'testRequest' key with an
 # object with 'params' key with an empty array. When the host system sends a
 # request matching, this mock plugin replies to it with the 'response' object.
 # ===-----------------------------------------------------------------------===#