From b29780e1d63f8a920a3e07b51fdb269bd495f2f9 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Thu, 17 Aug 2023 16:28:56 -0700 Subject: [PATCH 1/2] Avoid instantiating SwiftASTContextForModule in get_executable_triple() (NFC) Instead we can directly query the triple of the module. rdar://113997661 --- .../TypeSystem/Swift/SwiftASTContext.cpp | 18 +---------- .../Werror/TestSwiftStripWerror.py | 22 ++++++------- .../config_macros/TestSwiftDedupMacros.py | 32 ++++++------------- 3 files changed, 20 insertions(+), 52 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index fa19a00d465fb..b070870d75c16 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -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(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( @@ -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; diff --git a/lldb/test/API/lang/swift/clangimporter/Werror/TestSwiftStripWerror.py b/lldb/test/API/lang/swift/clangimporter/Werror/TestSwiftStripWerror.py index 1929ef37faad5..7d7373c808788 100644 --- a/lldb/test/API/lang/swift/clangimporter/Werror/TestSwiftStripWerror.py +++ b/lldb/test/API/lang/swift/clangimporter/Werror/TestSwiftStripWerror.py @@ -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( @@ -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) 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 diff --git a/lldb/test/API/lang/swift/clangimporter/config_macros/TestSwiftDedupMacros.py b/lldb/test/API/lang/swift/clangimporter/config_macros/TestSwiftDedupMacros.py index 7e1526288214d..d6d46afa17d94 100644 --- a/lldb/test/API/lang/swift/clangimporter/config_macros/TestSwiftDedupMacros.py +++ b/lldb/test/API/lang/swift/clangimporter/config_macros/TestSwiftDedupMacros.py @@ -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 From ca581896955165d1ec328bb545beb0779bf0aef5 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Fri, 18 Aug 2023 13:11:16 -0700 Subject: [PATCH 2/2] Apply dSYM path remapping dictionary also to the expression context. This fixes an oversight that was uncovered by making it less likely to instantiate a per-module SwiftASTContext. --- .../TypeSystem/Swift/SwiftASTContext.cpp | 36 ++++++++++--------- .../remap_sdk_path/TestSwiftRemapSDKPath.py | 13 ++----- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index b070870d75c16..7c396409e21de 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -2383,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 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 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 @@ -2421,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( diff --git a/lldb/test/API/lang/swift/clangimporter/remap_sdk_path/TestSwiftRemapSDKPath.py b/lldb/test/API/lang/swift/clangimporter/remap_sdk_path/TestSwiftRemapSDKPath.py index f0f7e5dad47b1..9b9bc80a6a603 100644 --- a/lldb/test/API/lang/swift/clangimporter/remap_sdk_path/TestSwiftRemapSDKPath.py +++ b/lldb/test/API/lang/swift/clangimporter/remap_sdk_path/TestSwiftRemapSDKPath.py @@ -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/