Skip to content

[lldb] Fix SwiftASTContext creatiion when swift caching is used #9137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 77 additions & 17 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions lldb/test/API/lang/swift/clangimporter/caching/Makefile
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions lldb/test/API/lang/swift/clangimporter/caching/a.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#include "b.h"

struct A {
struct B b;
};
1 change: 1 addition & 0 deletions lldb/test/API/lang/swift/clangimporter/caching/b.h
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
struct B {};
3 changes: 3 additions & 0 deletions lldb/test/API/lang/swift/clangimporter/caching/main.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import ClangA
let obj = A()
print("break here \(obj)")
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module ClangA {
header "a.h"
}

module ClangB {
header "b.h"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions lldb/unittests/Symbol/TestSwiftASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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"}));
Expand Down