Merge pull request swiftlang#9137 from cachemeifyoucan/eng/PR-134090397

[lldb] Fix SwiftASTContext creatiion when swift caching is used

Author: adrian-prantl

lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp

@@ -57,6 +57,7 @@
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Lex/Preprocessor.h"
 
 #include "clang/Lex/PreprocessorOptions.h"
@@ -1573,21 +1574,7 @@ bool ShouldUnique(StringRef arg) {
 
 // static
 void SwiftASTContext::AddExtraClangArgs(const std::vector<std::string> &source,
-                                        std::vector<std::string> &dest,
-                                        bool cc1) {
-  // FIXME: Support for cc1 flags isn't complete.  The uniquing
-  // algortihm below does not work for cc1 flags. Since cc1 flags are
-  // not stable it's not feasible to keep a list of all multi-arg
-  // flags, for example. It also makes it difficult to correctly
-  // identify where workng directories and path remappings should
-  // applied. For all these reasons, using cc1 flags for anything but
-  // a local build with explicit modules and precise compiler
-  // invocations isn't supported yet.
-  if (cc1) {
-    dest.insert(dest.end(), source.begin(), source.end());
-    return;
-  }
-
+                                        std::vector<std::string> &dest) {
   llvm::StringSet<> unique_flags;
   for (auto &arg : dest)
     unique_flags.insert(arg);
@@ -1779,8 +1766,14 @@ void SwiftASTContext::AddExtraClangArgs(
         eSeverityWarning,
         "Mixing and matching of driver and cc1 Clang options detected");
 
-  AddExtraClangArgs(ExtraArgs, importer_options.ExtraArgs,
-                    importer_options.DirectClangCC1ModuleBuild);
+  // If using direct cc1 flags, compute the arguments and return.
+  // Since this is cc1 flags, no driver overwrite can be applied.
+  if (importer_options.DirectClangCC1ModuleBuild) {
+    AddExtraClangCC1Args(ExtraArgs, importer_options.ExtraArgs);
+    return;
+  }
+
+  AddExtraClangArgs(ExtraArgs, importer_options.ExtraArgs);
   applyOverrideOptions(importer_options.ExtraArgs, overrideOpts);
   if (HasNonexistentExplicitModule(importer_options.ExtraArgs))
     RemoveExplicitModules(importer_options.ExtraArgs);
@@ -1792,6 +1785,73 @@ void SwiftASTContext::AddExtraClangArgs(
       });
 }
 
+void SwiftASTContext::AddExtraClangCC1Args(
+    const std::vector<std::string> &source, std::vector<std::string> &dest) {
+  clang::CompilerInvocation invocation;
+  llvm::SmallVector<const char *> clangArgs;
+  clangArgs.reserve(source.size());
+  llvm::for_each(source, [&](const std::string &Arg) {
+    // Workaround for the extra driver argument embedded in the swiftmodule by
+    // some swift compiler version. It always starts with `--target=` and it is
+    // not a valid cc1 option.
+    if (!StringRef(Arg).starts_with("--target="))
+      clangArgs.push_back(Arg.c_str());
+  });
+
+  std::string diags;
+  llvm::raw_string_ostream os(diags);
+  auto diagOpts = llvm::makeIntrusiveRefCnt<clang::DiagnosticOptions>();
+  clang::DiagnosticsEngine clangDiags(
+      new clang::DiagnosticIDs(), diagOpts,
+      new clang::TextDiagnosticPrinter(os, diagOpts.get()));
+
+  if (!clang::CompilerInvocation::CreateFromArgs(invocation, clangArgs,
+                                                 clangDiags)) {
+    // If cc1 arguments failed to parse, report diagnostics and return
+    // immediately.
+    AddDiagnostic(eSeverityError, diags);
+    // Disable direct-cc1 build as fallback.
+    GetClangImporterOptions().DirectClangCC1ModuleBuild = false;
+    return;
+  }
+
+  // Clear module cache key and other CAS options to load modules from disk
+  // directly.
+  invocation.getFrontendOpts().ModuleCacheKeys.clear();
+  invocation.getCASOpts() = clang::CASOptions();
+
+  // Remove non-existing modules in a systematic way.
+  bool module_missing = false;
+  auto CheckFileExists = [&](const char *file) {
+    if (!llvm::sys::fs::exists(file)) {
+      std::string m_description;
+      HEALTH_LOG_PRINTF("Nonexistent explicit module file %s", file);
+      module_missing = true;
+    }
+  };
+  llvm::for_each(invocation.getHeaderSearchOpts().PrebuiltModuleFiles,
+                 [&](const auto &mod) { CheckFileExists(mod.second.c_str()); });
+  llvm::for_each(invocation.getFrontendOpts().ModuleFiles,
+                 [&](const auto &mod) { CheckFileExists(mod.c_str()); });
+
+  // If missing, clear all the prebuilt module options and use implicit module
+  // build.
+  if (module_missing) {
+    invocation.getHeaderSearchOpts().PrebuiltModuleFiles.clear();
+    invocation.getFrontendOpts().ModuleFiles.clear();
+    invocation.getLangOpts().ImplicitModules = true;
+    invocation.getHeaderSearchOpts().ImplicitModuleMaps = true;
+  }
+
+  invocation.generateCC1CommandLine(
+      [&](const llvm::Twine &arg) { dest.push_back(arg.str()); });
+
+  // If cc1 arguments are parsed and generated correctly, set explicitly-built
+  // module since only explicit module build can use direct cc1 mode.
+  m_has_explicit_modules = true;
+  return;
+}
+
 void SwiftASTContext::AddUserClangArgs(TargetProperties &props) {
   Args args(props.GetSwiftExtraClangFlags());
   std::vector<std::string> user_clang_flags;

lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.h

@@ -278,8 +278,10 @@ class SwiftASTContext : public TypeSystemSwift {
   /// apply the working directory to any relative paths.
   void AddExtraClangArgs(const std::vector<std::string> &ExtraArgs,
                          llvm::StringRef overrideOpts = "");
+  void AddExtraClangCC1Args(const std::vector<std::string>& source,
+                                std::vector<std::string>& dest);
   static void AddExtraClangArgs(const std::vector<std::string>& source,
-                                std::vector<std::string>& dest, bool cc1);
+                                std::vector<std::string>& dest);
   static std::string GetPluginServer(llvm::StringRef plugin_library_path);
   /// Removes nonexisting VFS overlay options.
   static void FilterClangImporterOptions(std::vector<std::string> &extra_args,

lldb/test/API/lang/swift/clangimporter/caching/Makefile

@@ -0,0 +1,5 @@
+SWIFT_SOURCES := main.swift
+SWIFT_ENABLE_EXPLICIT_MODULES := YES
+SWIFTFLAGS_EXTRAS = -I$(SRCDIR) -cache-compile-job -cas-path $(BUILDDIR)/cas
+
+include Makefile.rules

lldb/test/API/lang/swift/clangimporter/caching/TestSwiftClangImporterCaching.py

@@ -0,0 +1,32 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+import unittest2
+
+class TestSwiftClangImporterCaching(TestBase):
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    # Don't run ClangImporter tests if Clangimporter is disabled.
+    @skipIf(setting=('symbols.use-swift-clangimporter', 'false'))
+    @skipIf(setting=('symbols.swift-precise-compiler-invocation', 'false'))
+    @skipIf(setting=('plugin.typesystem.clang.experimental-redecl-completion', 'true'), bugnumber='rdar://128094135')
+    @skipUnlessDarwin
+    @swiftTest
+    def test(self):
+        """
+        Test flipping on/off implicit modules.
+        """
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "break here",
+                                          lldb.SBFileSpec('main.swift'))
+        log = self.getBuildArtifact("types.log")
+        self.expect('log enable lldb types -f "%s"' % log)
+        self.expect("expression obj", DATA_TYPES_DISPLAYED_CORRECTLY,
+                    substrs=["b ="])
+        self.filecheck('platform shell cat "%s"' % log, __file__)
+### -cc1 should be round-tripped so there is no more `-cc1` in the extra args. Look for `-triple` which is a cc1 flag.
+#       CHECK:  SwiftASTContextForExpressions(module: "a", cu: "main.swift")::LogConfiguration() --     -triple
+#       CHECK-NOT: -cc1
+#       CHECK-NOT: -fmodule-file-cache-key

lldb/test/API/lang/swift/clangimporter/caching/a.h

@@ -0,0 +1,5 @@
+#include "b.h"
+
+struct A {
+  struct B b;
+};

lldb/test/API/lang/swift/clangimporter/caching/b.h

@@ -0,0 +1 @@
+struct B {};

lldb/test/API/lang/swift/clangimporter/caching/main.swift

@@ -0,0 +1,3 @@
+import ClangA
+let obj = A()
+print("break here \(obj)")

lldb/test/API/lang/swift/clangimporter/caching/module.modulemap

@@ -0,0 +1,7 @@
+module ClangA {
+  header "a.h"
+}
+
+module ClangB {
+  header "b.h"
+}

lldb/test/API/lang/swift/clangimporter/explicit_cc1/TestSwiftClangImporterExplicitCC1.py

@@ -26,4 +26,6 @@ def test(self):
         self.expect("expression obj", DATA_TYPES_DISPLAYED_CORRECTLY,
                     substrs=["b ="])
         self.filecheck('platform shell cat "%s"' % log, __file__)
-#       CHECK:  SwiftASTContextForExpressions(module: "a", cu: "main.swift")::LogConfiguration() --     -cc1
+### -cc1 should be round-tripped so there is no more `-cc1` in the extra args. Look for `-triple` which is a cc1 flag.
+#       CHECK:  SwiftASTContextForExpressions(module: "a", cu: "main.swift")::LogConfiguration() --     -triple
+#       CHECK-NOT: -cc1

lldb/unittests/Symbol/TestSwiftASTContext.cpp

@@ -234,15 +234,15 @@ const std::vector<std::string> uniqued_flags = {
 TEST_F(ClangArgs, UniquingCollisionWithExistingFlags) {
   const std::vector<std::string> source = duplicated_flags;
   std::vector<std::string> dest = uniqued_flags;
-  SwiftASTContext::AddExtraClangArgs(source, dest, false);
+  SwiftASTContext::AddExtraClangArgs(source, dest);
 
   EXPECT_EQ(dest, uniqued_flags);
 }
 
 TEST_F(ClangArgs, UniquingCollisionWithAddedFlags) {
   const std::vector<std::string> source = duplicated_flags;
   std::vector<std::string> dest;
-  SwiftASTContext::AddExtraClangArgs(source, dest, false);
+  SwiftASTContext::AddExtraClangArgs(source, dest);
 
   EXPECT_EQ(dest, uniqued_flags);
 }
@@ -251,7 +251,7 @@ TEST_F(ClangArgs, DoubleDash) {
   // -v with all currently ignored arguments following.
   const std::vector<std::string> source{"-v", "--", "-Werror", ""};
   std::vector<std::string> dest;
-  SwiftASTContext::AddExtraClangArgs(source, dest, false);
+  SwiftASTContext::AddExtraClangArgs(source, dest);
 
   // Check that all ignored arguments got removed.
   EXPECT_EQ(dest, std::vector<std::string>({"-v"}));