Merge pull request #70456 from cachemeifyoucan/eng/PR-119577349

[ClangImporter] Consolidate clang invocation creation

Author: cachemeifyoucan

include/swift/AST/DiagnosticsClangImporter.def

@@ -117,6 +117,7 @@ WARNING(nonmutating_without_mutable_fields,none,
         "attribute 'nonmutating' has no effect without any mutable fields", ())
 
 ERROR(module_map_not_found, none, "module map file '%0' not found", (StringRef))
+WARNING(module_map_ignored, none, "module map file '%0' will be ignored", (StringRef))
 
 WARNING(libc_not_found, none,
         "libc not found for '%0'; C stdlib may be unavailable",

include/swift/Basic/LangOptions.h

@@ -960,10 +960,6 @@ namespace swift {
     /// DWARFImporter delegate.
     bool DisableSourceImport = false;
 
-    /// When set, use ExtraArgs alone to configure clang instance because ExtraArgs
-    /// contains the full option set.
-    bool ExtraArgsOnly = false;
-
     /// When building a PCM, rely on the Swift frontend's command-line -Xcc flags
     /// to build the Clang module via Clang frontend directly,
     /// and completely bypass the Clang driver.

include/swift/ClangImporter/ClangImporter.h

@@ -176,14 +176,19 @@ class ClangImporter final : public ClangModuleLoader {
          DWARFImporterDelegate *dwarfImporterDelegate = nullptr);
 
   static std::vector<std::string>
-  getClangArguments(ASTContext &ctx, bool ignoreClangTarget = false);
+  getClangDriverArguments(ASTContext &ctx, bool ignoreClangTarget = false);
+
+  static std::optional<std::vector<std::string>>
+  getClangCC1Arguments(ClangImporter *importer, ASTContext &ctx,
+                       llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+                       bool ignoreClangTarget = false);
 
   static std::unique_ptr<clang::CompilerInvocation>
   createClangInvocation(ClangImporter *importer,
                         const ClangImporterOptions &importerOpts,
                         llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
-                        ArrayRef<std::string> invocationArgStrs,
-                        std::vector<std::string> *CC1Args = nullptr);
+                        std::vector<std::string> &CC1Args);
+
   ClangImporter(const ClangImporter &) = delete;
   ClangImporter(ClangImporter &&) = delete;
   ClangImporter &operator=(const ClangImporter &) = delete;

include/swift/Option/FrontendOptions.td

@@ -1115,9 +1115,6 @@ def target_variant_sdk_version : Separate<["-"], "target-variant-sdk-version">,
 def target_sdk_name : Separate<["-"], "target-sdk-name">,
   HelpText<"Canonical name of the target SDK used for compilation">;
 
-def extra_clang_options_only : Flag<["-"], "only-use-extra-clang-opts">,
-  HelpText<"Options passed via -Xcc are sufficient for Clang configuration">;
-
 def candidate_module_file
   : Separate<["-"], "candidate-module-file">, MetaVarName<"<path>">,
     HelpText<"Specify Swift module may be ready to use for an interface">;

lib/ClangImporter/ClangImporter.cpp

@@ -52,13 +52,15 @@
 #include "swift/Subsystems.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Mangle.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
 #include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Preprocessor.h"
@@ -85,6 +87,7 @@
 #include "llvm/TextAPI/TextAPIReader.h"
 #include <algorithm>
 #include <memory>
+#include <optional>
 #include <string>
 
 using namespace swift;
@@ -1040,19 +1043,13 @@ ClangImporter::getOrCreatePCH(const ClangImporterOptions &ImporterOptions,
 }
 
 std::vector<std::string>
-ClangImporter::getClangArguments(ASTContext &ctx, bool ignoreClangTarget) {
+ClangImporter::getClangDriverArguments(ASTContext &ctx, bool ignoreClangTarget) {
+  assert(!ctx.ClangImporterOpts.DirectClangCC1ModuleBuild &&
+         "direct-clang-cc1-module-build should not call this function");
   std::vector<std::string> invocationArgStrs;
   // When creating from driver commands, clang expects this to be like an actual
   // command line. So we need to pass in "clang" for argv[0]
-  if (!ctx.ClangImporterOpts.DirectClangCC1ModuleBuild)
-    invocationArgStrs.push_back(ctx.ClangImporterOpts.clangPath);
-  if (ctx.ClangImporterOpts.ExtraArgsOnly) {
-    invocationArgStrs.insert(invocationArgStrs.end(),
-                             ctx.ClangImporterOpts.ExtraArgs.begin(),
-                             ctx.ClangImporterOpts.ExtraArgs.end());
-    return invocationArgStrs;
-  }
-
+  invocationArgStrs.push_back(ctx.ClangImporterOpts.clangPath);
   switch (ctx.ClangImporterOpts.Mode) {
   case ClangImporterOptions::Modes::Normal:
   case ClangImporterOptions::Modes::PrecompiledModule:
@@ -1066,69 +1063,59 @@ ClangImporter::getClangArguments(ASTContext &ctx, bool ignoreClangTarget) {
   return invocationArgStrs;
 }
 
-std::unique_ptr<clang::CompilerInvocation> ClangImporter::createClangInvocation(
-    ClangImporter *importer, const ClangImporterOptions &importerOpts,
+std::optional<std::vector<std::string>> ClangImporter::getClangCC1Arguments(
+    ClangImporter *importer, ASTContext &ctx,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
-    ArrayRef<std::string> invocationArgStrs,
-    std::vector<std::string> *CC1Args) {
-  std::vector<const char *> invocationArgs;
-  invocationArgs.reserve(invocationArgStrs.size());
-  for (auto &argStr : invocationArgStrs)
-    invocationArgs.push_back(argStr.c_str());
-
-  llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine> clangDiags;
-  std::unique_ptr<clang::CompilerInvocation> CI;
-  if (importerOpts.DirectClangCC1ModuleBuild) {
-    // In this mode, we bypass createInvocationFromCommandLine, which goes
-    // through the Clang driver, and use strictly cc1 arguments to instantiate a
-    // clang Instance directly, assuming that the set of '-Xcc <X>' frontend flags is
-    // fully sufficient to do so.
-
-    // Because we are bypassing the Clang driver, we must populate
-    // the diagnostic options here explicitly.
-    std::unique_ptr<clang::DiagnosticOptions> clangDiagOpts =
-        clang::CreateAndPopulateDiagOpts(invocationArgs);
-    auto *diagClient = new ClangDiagnosticConsumer(
-        importer->Impl, *clangDiagOpts, importerOpts.DumpClangDiagnostics);
-    clangDiags = clang::CompilerInstance::createDiagnostics(
-        clangDiagOpts.release(), diagClient,
-        /*owned*/ true);
-
-    // Finally, use the CC1 command-line and the diagnostic engine
-    // to instantiate our Invocation.
-    CI = std::make_unique<clang::CompilerInvocation>();
-    if (!clang::CompilerInvocation::CreateFromArgs(
-        *CI, invocationArgs, *clangDiags, invocationArgs[0]))
-      return nullptr;
-  } else {
-    // Set up a temporary diagnostic client to report errors from parsing the
-    // command line, which may be important for Swift clients if, for example,
-    // they're using -Xcc options. Unfortunately this diagnostic engine has to
-    // use the default options because the /actual/ options haven't been parsed
-    // yet.
-    //
-    // The long-term client for Clang diagnostics is set up below, after the
-    // clang::CompilerInstance is created.
-    llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> tempDiagOpts{
-        new clang::DiagnosticOptions};
+    bool ignoreClangTarget) {
+  // If using direct cc1 module build, return extra args only.
+  if (ctx.ClangImporterOpts.DirectClangCC1ModuleBuild)
+    return ctx.ClangImporterOpts.ExtraArgs;
+
+  // Otherwise, create cc1 arguments from driver args.
+  auto driverArgs = getClangDriverArguments(ctx, ignoreClangTarget);
+
+  llvm::SmallVector<const char *> invocationArgs;
+  invocationArgs.reserve(driverArgs.size());
+  llvm::for_each(driverArgs, [&](const std::string &Arg) {
+    invocationArgs.push_back(Arg.c_str());
+  });
 
-    auto *tempDiagClient = new ClangDiagnosticConsumer(
-        importer->Impl, *tempDiagOpts, importerOpts.DumpClangDiagnostics);
-    clangDiags = clang::CompilerInstance::createDiagnostics(tempDiagOpts.get(),
-                                                            tempDiagClient,
-                                                            /*owned*/ true);
-    clang::CreateInvocationOptions CIOpts;
-    CIOpts.VFS = VFS;
-    CIOpts.Diags = clangDiags;
-    CIOpts.RecoverOnError = false;
-    CIOpts.CC1Args = CC1Args;
-    CIOpts.ProbePrecompiled = true;
-    CI = clang::createInvocation(invocationArgs, std::move(CIOpts));
+  if (ctx.ClangImporterOpts.DumpClangDiagnostics) {
+    llvm::errs() << "clang importer driver args: '";
+    llvm::interleave(
+        invocationArgs, [](StringRef arg) { llvm::errs() << arg; },
+        [] { llvm::errs() << "' '"; });
+    llvm::errs() << "'\n";
   }
 
-  if (!CI) {
-    return CI;
-  }
+  // Set up a temporary diagnostic client to report errors from parsing the
+  // command line, which may be important for Swift clients if, for example,
+  // they're using -Xcc options. Unfortunately this diagnostic engine has to
+  // use the default options because the /actual/ options haven't been parsed
+  // yet.
+  //
+  // The long-term client for Clang diagnostics is set up afterwards, after the
+  // clang::CompilerInstance is created.
+  llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> tempDiagOpts{
+      new clang::DiagnosticOptions};
+
+  auto *tempDiagClient =
+      new ClangDiagnosticConsumer(importer->Impl, *tempDiagOpts,
+                                  ctx.ClangImporterOpts.DumpClangDiagnostics);
+
+  auto clangDiags = clang::CompilerInstance::createDiagnostics(
+      tempDiagOpts.get(), tempDiagClient,
+      /*owned*/ true);
+
+  clang::CreateInvocationOptions CIOpts;
+  CIOpts.VFS = VFS;
+  CIOpts.Diags = clangDiags;
+  CIOpts.RecoverOnError = false;
+  CIOpts.ProbePrecompiled = true;
+  auto CI = clang::createInvocation(invocationArgs, std::move(CIOpts));
+
+  if (!CI)
+    return std::nullopt;
 
   // FIXME: clang fails to generate a module if there is a `-fmodule-map-file`
   // argument pointing to a missing file.
@@ -1146,7 +1133,12 @@ std::unique_ptr<clang::CompilerInvocation> ClangImporter::createClangInvocation(
 
   std::vector<std::string> FilteredModuleMapFiles;
   for (auto ModuleMapFile : CI->getFrontendOpts().ModuleMapFiles) {
-    if (TempVFS->exists(ModuleMapFile)) {
+    if (ctx.ClangImporterOpts.UseClangIncludeTree) {
+      // There is no need to add any module map file here. Issue a warning and
+      // drop the option.
+      importer->Impl.diagnose(SourceLoc(), diag::module_map_ignored,
+                              ModuleMapFile);
+    } else if (TempVFS->exists(ModuleMapFile)) {
       FilteredModuleMapFiles.push_back(ModuleMapFile);
     } else {
       importer->Impl.diagnose(SourceLoc(), diag::module_map_not_found,
@@ -1155,6 +1147,35 @@ std::unique_ptr<clang::CompilerInvocation> ClangImporter::createClangInvocation(
   }
   CI->getFrontendOpts().ModuleMapFiles = FilteredModuleMapFiles;
 
+  return CI->getCC1CommandLine();
+}
+
+std::unique_ptr<clang::CompilerInvocation> ClangImporter::createClangInvocation(
+    ClangImporter *importer, const ClangImporterOptions &importerOpts,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+    std::vector<std::string> &CC1Args) {
+  std::vector<const char *> invocationArgs;
+  invocationArgs.reserve(CC1Args.size());
+  llvm::for_each(CC1Args, [&](const std::string &Arg) {
+    invocationArgs.push_back(Arg.c_str());
+  });
+
+  // Create a diagnostics engine for creating clang compiler invocation. The
+  // option here is either generated by dependency scanner or just round tripped
+  // from `getClangCC1Arguments` so we don't expect it to fail. Use a simple
+  // printing diagnostics consumer for debugging any unexpected error.
+  auto diagOpts = llvm::makeIntrusiveRefCnt<clang::DiagnosticOptions>();
+  clang::DiagnosticsEngine clangDiags(
+      new clang::DiagnosticIDs(), diagOpts,
+      new clang::TextDiagnosticPrinter(llvm::errs(), diagOpts.get()));
+
+  // Finally, use the CC1 command-line and the diagnostic engine
+  // to instantiate our Invocation.
+  auto CI = std::make_unique<clang::CompilerInvocation>();
+  if (!clang::CompilerInvocation::CreateFromArgs(
+          *CI, invocationArgs, clangDiags, importerOpts.clangPath.c_str()))
+    return nullptr;
+
   return CI;
 }
 
@@ -1204,21 +1225,24 @@ ClangImporter::create(ASTContext &ctx,
 
   // Create a new Clang compiler invocation.
   {
-    importer->Impl.ClangArgs = getClangArguments(ctx);
-    if (fileMapping.requiresBuiltinHeadersInSystemModules) {
-      importer->Impl.ClangArgs.push_back("-Xclang");
+    if (auto ClangArgs = getClangCC1Arguments(importer.get(), ctx, VFS))
+      importer->Impl.ClangArgs = *ClangArgs;
+    else
+      return nullptr;
+
+    if (fileMapping.requiresBuiltinHeadersInSystemModules)
       importer->Impl.ClangArgs.push_back("-fbuiltin-headers-in-system-modules");
-    }
+
     ArrayRef<std::string> invocationArgStrs = importer->Impl.ClangArgs;
     if (importerOpts.DumpClangDiagnostics) {
-      llvm::errs() << "'";
+      llvm::errs() << "clang importer cc1 args: '";
       llvm::interleave(
                        invocationArgStrs, [](StringRef arg) { llvm::errs() << arg; },
                        [] { llvm::errs() << "' '"; });
       llvm::errs() << "'\n";
     }
     importer->Impl.Invocation = createClangInvocation(
-        importer.get(), importerOpts, VFS, invocationArgStrs);
+        importer.get(), importerOpts, VFS, importer->Impl.ClangArgs);
     if (!importer->Impl.Invocation)
       return nullptr;
   }
@@ -1296,10 +1320,12 @@ ClangImporter::create(ASTContext &ctx,
   if (ctx.LangOpts.ClangTarget.has_value()) {
     // If '-clang-target' is set, create a mock invocation with the Swift triple
     // to configure CodeGen and Target options for Swift compilation.
-    auto swiftTargetClangArgs = getClangArguments(ctx, true);
-    ArrayRef<std::string> invocationArgStrs = swiftTargetClangArgs;
+    auto swiftTargetClangArgs =
+        getClangCC1Arguments(importer.get(), ctx, VFS, true);
+    if (!swiftTargetClangArgs)
+      return nullptr;
     auto swiftTargetClangInvocation = createClangInvocation(
-        importer.get(), importerOpts, VFS, invocationArgStrs);
+        importer.get(), importerOpts, VFS, *swiftTargetClangArgs);
     if (!swiftTargetClangInvocation)
       return nullptr;
     importer->Impl.setSwiftTargetInfo(clang::TargetInfo::CreateTargetInfo(

lib/ClangImporter/ClangModuleDependencyScanner.cpp

@@ -86,7 +86,7 @@ static void addSearchPathInvocationArguments(
 static std::vector<std::string> getClangDepScanningInvocationArguments(
     ASTContext &ctx, llvm::Optional<StringRef> sourceFileName = llvm::None) {
   std::vector<std::string> commandLineArgs =
-      ClangImporter::getClangArguments(ctx);
+      ClangImporter::getClangDriverArguments(ctx);
   addSearchPathInvocationArguments(commandLineArgs, ctx);
 
   auto sourceFilePos = std::find(
@@ -167,10 +167,6 @@ ModuleDependencyVector ClangImporter::bridgeClangModuleDependencies(
     swiftArgs.push_back("-module-name");
     swiftArgs.push_back(clangModuleDep.ID.ModuleName);
 
-    // We pass the entire argument list via -Xcc, so the invocation should
-    // use extra clang options alone.
-    swiftArgs.push_back("-only-use-extra-clang-opts");
-
     auto pcmPath = moduleCacheRelativeLookupModuleOutput(
         clangModuleDep.ID, ModuleOutputKind::ModuleFile, moduleOutputPath);
     swiftArgs.push_back("-o");
@@ -307,10 +303,6 @@ void ClangImporter::recordBridgingHeaderOptions(
   // Swift frontend action: -emit-pcm
   swiftArgs.push_back("-emit-pch");
 
-  // We pass the entire argument list via -Xcc, so the invocation should
-  // use extra clang options alone.
-  swiftArgs.push_back("-only-use-extra-clang-opts");
-
   // Ensure that the resulting PCM build invocation uses Clang frontend
   // directly
   swiftArgs.push_back("-direct-clang-cc1-module-build");

lib/Frontend/CompilerInvocation.cpp

@@ -1655,7 +1655,6 @@ static bool ParseClangImporterArgs(ClangImporterOptions &Opts, ArgList &Args,
     Opts.EnableClangSPI = false;
   }
 
-  Opts.ExtraArgsOnly |= Args.hasArg(OPT_extra_clang_options_only);
   Opts.DirectClangCC1ModuleBuild |= Args.hasArg(OPT_direct_clang_cc1_module_build);
 
   if (const Arg *A = Args.getLastArg(OPT_pch_output_dir)) {

test/CAS/cas-explicit-module-map.swift

@@ -110,6 +110,11 @@
 
 // RUN: %target-swift-frontend -emit-module -emit-module-path %t/Foo.swiftmodule -disable-implicit-swift-modules -module-cache-path %t.module-cache -explicit-swift-module-map-file @%t/map.casid -Rmodule-loading -Xcc -Rmodule-import %s -cache-compile-job -cas-path %t/cas -allow-unstable-cache-key-for-testing 2>&1 | %FileCheck %s
 
+/// Test that if there are non-existing module-map file passed through -Xcc, this still compiles.
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/Foo.swiftmodule -disable-implicit-swift-modules -module-cache-path %t.module-cache -explicit-swift-module-map-file @%t/map.casid -Rmodule-loading -Xcc -Rmodule-import %s -cache-compile-job -cas-path %t/cas -allow-unstable-cache-key-for-testing -Xcc -fmodule-map-file=%t/do-not-exist.modulemap 2>&1 | %FileCheck %s --check-prefix=CHECK --check-prefix=MODULEMAP-IGNORE
+
+// MODULEMAP-IGNORE: warning: module map file '{{.*}}' will be ignored
+
 // RUN: %target-swift-frontend -typecheck -emit-module-interface-path %t/Foo.swiftinterface -disable-implicit-swift-modules -module-cache-path %t.module-cache -explicit-swift-module-map-file @%t/map.casid %s -cache-compile-job -cas-path %t/cas -allow-unstable-cache-key-for-testing -swift-version 5 -enable-library-evolution -o %t/Foo.swiftmodule
 // RUN: %cache-tool -cas-path %t/cas -cache-tool-action print-output-keys -- \
 // RUN:   %target-swift-frontend -typecheck -emit-module-interface-path %t/Foo.swiftinterface -disable-implicit-swift-modules \

test/ClangImporter/pcm-emit-direct-cc1-mode.swift

@@ -1,9 +1,6 @@
 // Emit the explicit module.
 // RUN: %empty-directory(%t)
-// RUN: %swift-frontend -emit-pcm -direct-clang-cc1-module-build -only-use-extra-clang-opts -module-name script -o %t/script.pcm %S/Inputs/custom-modules/module.modulemap -Xcc %S/Inputs/custom-modules/module.modulemap -Xcc -o -Xcc %t/script.pcm -Xcc -fmodules -Xcc -triple -Xcc %target-triple -Xcc -x -Xcc objective-c -dump-clang-diagnostics 2> %t.diags.txt
-
-// Sometimes returns a 1 exit code with no stdout or stderr or even an indication of which command failed.
-// XFAIL: OS=linux-gnu, OS=linux-android, OS=linux-androideabi
+// RUN: %swift-frontend -emit-pcm -direct-clang-cc1-module-build -module-name script -o %t/script.pcm %S/Inputs/custom-modules/module.modulemap -Xcc %S/Inputs/custom-modules/module.modulemap -Xcc -o -Xcc %t/script.pcm -Xcc -fmodules -Xcc -triple -Xcc %target-triple -Xcc -x -Xcc objective-c -dump-clang-diagnostics 2> %t.diags.txt
 
 // Verify some of the output of the -dump-pcm flag.
 // RUN: %swift-dump-pcm %t/script.pcm | %FileCheck %s --check-prefix=CHECK-DUMP
@@ -13,7 +10,9 @@
 
 // Verify that the clang command-line used is cc1
 // RUN: %FileCheck -check-prefix CHECK-CLANG -DTRIPLE=%target-triple %s < %t.diags.txt
-// CHECK-CLANG: '{{.*[/\\]}}module.modulemap' '-o' '{{.*[/\\]}}script.pcm' '-fmodules' '-triple' '[[TRIPLE]]' '-x' 'objective-c'
+// CHECK-CLANG: clang importer cc1 args
+// CHECK-CLANG-SAME: '{{.*[/\\]}}module.modulemap' '-o' '{{.*[/\\]}}script.pcm' '-fmodules' '-triple' '[[TRIPLE]]' '-x' 'objective-c'
+// CHECK-CLANG-NOT: clang importer driver args
 
 import script
 var _ : ScriptTy

test/ScanDependencies/module_deps.swift

@@ -103,7 +103,7 @@ import SubE
 
 // CHECK: "commandLine": [
 // CHECK-NEXT: "-frontend"
-// CHECK-NEXT: "-only-use-extra-clang-opts"
+// CHECK: "-direct-clang-cc1-module-build"
 // CHECK-NOT:  "BUILD_DIR/bin/clang"
 // CHECK:      "-Xcc"
 // CHECK-NEXT: "-resource-dir"