Skip to content

Avoid instantiating SwiftASTContextForModule in get_executable_triple… #7265

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.

Sign up for GitHub

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 2 commits into from
Aug 19, 2023
Merged

Avoid instantiating SwiftASTContextForModule in get_executable_triple… #7265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b29780e
Avoid instantiating SwiftASTContextForModule in get_executable_triple…
adrian-prantl Aug 17, 2023
ca58189
Apply dSYM path remapping dictionary also to the expression context.
adrian-prantl Aug 18, 2023
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
54 changes: 20 additions & 34 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2064,19 +2064,6 @@ static lldb::ModuleSP GetUnitTestModule(lldb_private::ModuleList &modules) {
return ModuleSP();
}

static SwiftASTContext *GetModuleSwiftASTContext(Module &module) {
auto type_system_or_err =
module.GetTypeSystemForLanguage(lldb::eLanguageTypeSwift);
if (!type_system_or_err) {
llvm::consumeError(type_system_or_err.takeError());
return {};
}
auto *ts = llvm::dyn_cast_or_null<TypeSystemSwift>(type_system_or_err->get());
if (!ts)
return {};
return ts->GetSwiftASTContext();
}

/// Scan a newly added lldb::Module for Swift modules and report any errors in
/// its module SwiftASTContext to Target.
static void ProcessModule(
Expand Down Expand Up @@ -2313,10 +2300,7 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
auto get_executable_triple = [&]() -> llvm::Triple {
if (!exe_module_sp)
return {};
auto *exe_ast_ctx = GetModuleSwiftASTContext(*exe_module_sp);
if (!exe_ast_ctx)
return {};
return exe_ast_ctx->GetLanguageOptions().Target;
return exe_module_sp->GetArchitecture().GetTriple();
};

llvm::Triple computed_triple;
Expand Down Expand Up @@ -2399,25 +2383,22 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
const bool use_all_compiler_flags =
!got_serialized_options || target.GetUseAllCompilerFlags();

for (size_t mi = 0; mi != num_images; ++mi) {
std::vector<std::string> extra_clang_args;
ProcessModule(target.GetImages().GetModuleAtIndex(mi), m_description,
discover_implicit_search_paths, use_all_compiler_flags,
target, triple, plugin_search_options, module_search_paths,
framework_search_paths, extra_clang_args);
swift_ast_sp->AddExtraClangArgs(extra_clang_args);
}
for (ModuleSP module_sp : target.GetImages().Modules())
if (module_sp) {
std::vector<std::string> extra_clang_args;
ProcessModule(module_sp, m_description, discover_implicit_search_paths,
use_all_compiler_flags, target, triple,
plugin_search_options, module_search_paths,
framework_search_paths, extra_clang_args);
swift_ast_sp->AddExtraClangArgs(extra_clang_args);
}

FileSpecList target_module_paths = target.GetSwiftModuleSearchPaths();
for (size_t mi = 0, me = target_module_paths.GetSize(); mi != me; ++mi)
module_search_paths.push_back(
target_module_paths.GetFileSpecAtIndex(mi).GetPath());
for (const FileSpec &path : target.GetSwiftModuleSearchPaths())
module_search_paths.push_back(path.GetPath());

FileSpecList target_framework_paths = target.GetSwiftFrameworkSearchPaths();
for (size_t fi = 0, fe = target_framework_paths.GetSize(); fi != fe; ++fi)
framework_search_paths.push_back(
{target_framework_paths.GetFileSpecAtIndex(fi).GetPath(),
/*is_system*/ false});
for (const FileSpec &path : target.GetSwiftFrameworkSearchPaths())
framework_search_paths.push_back({path.GetPath(),
/*is_system*/ false});

// Now fold any extra options we were passed. This has to be done
// BEFORE the ClangImporter is made by calling GetClangImporter or
Expand All @@ -2437,6 +2418,11 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(

swift_ast_sp->ApplyDiagnosticOptions();

// Apply source path remappings found in each module's dSYM.
for (ModuleSP module : target.GetImages().Modules())
if (module)
swift_ast_sp->RemapClangImporterOptions(module->GetSourceMappingList());

// Apply source path remappings found in the target settings.
swift_ast_sp->RemapClangImporterOptions(target.GetSourcePathMap());
swift_ast_sp->FilterClangImporterOptions(
Expand Down
22 changes: 9 additions & 13 deletions lldb/test/API/lang/swift/clangimporter/Werror/TestSwiftStripWerror.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def setUp(self):
@swiftTest
def test(self):
"""This tests that -Werror is removed from ClangImporter options by
introducing two conflicting macro definitions in idfferent dylibs.
introducing two conflicting macro definitions in different dylibs.
"""
self.build()
target, _, _, _ = lldbutil.run_to_source_breakpoint(
Expand All @@ -26,17 +26,13 @@ def test(self):

# Turn on logging.
log = self.getBuildArtifact("types.log")
self.expect("log enable lldb types -f " + log)
self.expect('log enable lldb types -f "%s"' % log)
Copy link

@kastiglione kastiglione Aug 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can use python f-strings now:

Suggested change
self.expect('log enable lldb types -f "%s"' % log)
self.expect(f'log enable lldb types -f "{log}"')

Copy link
Author

@adrian-prantl adrian-prantl Aug 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I did not know about this feature!


self.expect("expression foo", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["42"])
sanity = 0
import io

logfile = io.open(log, "r", encoding="utf-8")
for line in logfile:
self.assertFalse("-Werror" in line)
if "-DCONFLICT" in line:
sanity += 1
# We see -DCONFLICT twice in the expression context and once in each of
# the two Module contexts.
self.assertEqual(sanity, 2 + 2)
self.filecheck('platform shell cat "%s"' % log, __file__)
# CHECK-NOT: SwiftASTContextForExpressions{{.*}}-Werror
# CHECK: SwiftASTContextForExpressions{{.*}}-DCONFLICT
# CHECK-NOT: SwiftASTContextForExpressions{{.*}}-Werror
# CHECK: SwiftASTContextForExpressions{{.*}}-DCONFLICT
# CHECK-NOT: SwiftASTContextForExpressions{{.*}}-DCONFLICT
# CHECK-NOT: SwiftASTContextForExpressions{{.*}}-Werror
32 changes: 10 additions & 22 deletions lldb/test/API/lang/swift/clangimporter/config_macros/TestSwiftDedupMacros.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,15 @@ def testSwiftDebugMacros(self):

# Turn on logging.
log = self.getBuildArtifact("types.log")
self.expect("log enable lldb types -f " + log)
self.expect('log enable lldb types -f ""%s"' % log)

self.expect("expression foo", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["42"])
debug = 0
space = 0
ndebug = 0
space_with_space = 0
import io

logfile = io.open(log, "r", encoding="utf-8")
for line in logfile:
if "-DDEBUG=1" in line:
debug += 1
if "-DSPACE" in line:
space += 1
if " SPACE" in line:
space_with_space += 1
if "-UNDEBUG" in line:
ndebug += 1
# One extra in SwiftASTContextPerModule.
self.assertEqual(debug, 3)
self.assertEqual(space, 3)
self.assertEqual(space_with_space, 0)
self.assertEqual(ndebug, 3)
self.filecheck('platform shell cat "%s"' % log, __file__)
# CHECK: SwiftASTContextForExpressions{{.*}}-DDEBUG=1
# CHECK: SwiftASTContextForExpressions{{.*}}-DSPACE
# CHECK-NOT: {{ SPACE}}
# CHECK: SwiftASTContextForExpressions{{.*}}-UNDEBUG
# CHECK: SwiftASTContextForModule("libDylib{{.*}}-DDEBUG=1
# CHECK: SwiftASTContextForModule("libDylib{{.*}}-DSPACE
# CHECK-NOT: {{ SPACE}}
# CHECK: SwiftASTContextForModule("libDylib{{.*}}-UNDEBUG
13 changes: 2 additions & 11 deletions lldb/test/API/lang/swift/clangimporter/remap_sdk_path/TestSwiftRemapSDKPath.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,5 @@ def test(self):
self.expect("expression 1", substrs=["1"])

# Scan through the types log.
import io

logfile = io.open(log, "r", encoding="utf-8")
found = 0
for line in logfile:
if line.startswith(
' SwiftASTContextForModule("a.out")::RemapClangImporterOptions() -- remapped'
):
if "/LocalSDK/" in line:
found += 1
self.assertEqual(found, 1)
self.filecheck('platform shell cat "%s"' % log, __file__)
# CHECK: SwiftASTContextForExpressions::RemapClangImporterOptions() -- remapped{{.*}}/LocalSDK/