From 00c0668177101840fa6a0a79e5058882de097cfd Mon Sep 17 00:00:00 2001 From: Dave Lee Date: Thu, 7 Mar 2024 11:21:21 -0800 Subject: [PATCH 1/9] [lldb] Fallback to implicit modules when explicit modules are missing --- .../TypeSystem/Swift/SwiftASTContext.cpp | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index da4eb759df5b3..11f47740c6522 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -57,6 +57,7 @@ #include "clang/Driver/Driver.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/CodeGen/TargetSubtargetInfo.h" @@ -64,6 +65,7 @@ #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" #include "llvm/MC/TargetRegistry.h" +#include "llvm/Support/FileSystem.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" #include "llvm/Support/TargetSelect.h" @@ -1594,9 +1596,50 @@ void SwiftASTContext::AddExtraClangArgs(const std::vector &source, } } +namespace { + +bool HasNonexistentExplicitModule(const std::vector &args) { + for (const auto &arg : args) { + StringRef val = arg; + if (!val.consume_front("-fmodule-file=")) + continue; + // The value's format is 'ModuleName=ModulePath'. Drop the prefix up to the + // first '=' character to obtain the path. + size_t eq = val.find('='); + assert(eq != std::string::npos); + if (eq == std::string::npos) + continue; + StringRef path = val.drop_front(eq + 1); + if (!llvm::sys::fs::exists(path)) { + std::string m_description; + HEALTH_LOG_PRINTF("Nonexistent explicit module file %s", path.data()); + return true; + } + } + return false; +} + +void RemoveExplicitModules(std::vector &args) { + // TODO: Should module map file arguments also be removed? Or is there a + // benefit to keeping "-fmodule-map-file=" options? + llvm::erase_if(args, [](const std::string &arg) { + // TODO: Maybe also "-fno-implicit-module-maps" + if (arg == "-fno-implicit-modules") + return true; + if (StringRef s = arg; s.starts_with("-fmodule-file=")) + return true; + + return false; + }); +} + +} // namespace + void SwiftASTContext::AddExtraClangArgs(const std::vector &ExtraArgs) { swift::ClangImporterOptions &importer_options = GetClangImporterOptions(); AddExtraClangArgs(ExtraArgs, importer_options.ExtraArgs); + if (HasNonexistentExplicitModule(importer_options.ExtraArgs)) + RemoveExplicitModules(importer_options.ExtraArgs); } void SwiftASTContext::AddUserClangArgs(TargetProperties &props) { From b4910887aee4836c0872133b81356ec689f3287a Mon Sep 17 00:00:00 2001 From: Dave Lee Date: Thu, 7 Mar 2024 12:49:58 -0800 Subject: [PATCH 2/9] Remove explicit module maps too --- lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index 11f47740c6522..b75e6f5e21482 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -1620,13 +1620,11 @@ bool HasNonexistentExplicitModule(const std::vector &args) { } void RemoveExplicitModules(std::vector &args) { - // TODO: Should module map file arguments also be removed? Or is there a - // benefit to keeping "-fmodule-map-file=" options? llvm::erase_if(args, [](const std::string &arg) { - // TODO: Maybe also "-fno-implicit-module-maps" - if (arg == "-fno-implicit-modules") + if (arg == "-fno-implicit-modules" || arg == "-fno-implicit-module-maps") return true; - if (StringRef s = arg; s.starts_with("-fmodule-file=")) + StringRef s = arg; + if (s.starts_with("-fmodule-file=") || s.starts_with("-fmodule-map-file=")) return true; return false; From 4ae9c2aeaac75e5b183e3460f205e92bb46c42a3 Mon Sep 17 00:00:00 2001 From: Dave Lee Date: Thu, 7 Mar 2024 13:30:23 -0800 Subject: [PATCH 3/9] Handle both formats of -fmodule-file= --- .../TypeSystem/Swift/SwiftASTContext.cpp | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index b75e6f5e21482..2cb9e1cd705ea 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -1600,17 +1600,19 @@ namespace { bool HasNonexistentExplicitModule(const std::vector &args) { for (const auto &arg : args) { - StringRef val = arg; - if (!val.consume_front("-fmodule-file=")) + StringRef value = arg; + if (!value.consume_front("-fmodule-file=")) continue; - // The value's format is 'ModuleName=ModulePath'. Drop the prefix up to the - // first '=' character to obtain the path. - size_t eq = val.find('='); - assert(eq != std::string::npos); - if (eq == std::string::npos) - continue; - StringRef path = val.drop_front(eq + 1); - if (!llvm::sys::fs::exists(path)) { + StringRef path = value; + size_t eq = value.find('='); + // The value that follows is in one of two formats: + // 1. ModuleName=ModulePath + // 2. ModulePath + if (eq != std::string::npos) + // The value appears to be in ModuleName=ModulePath forat. + path = value.drop_front(eq + 1); + // Check both path and value. This is to handle paths containing '='. + if (!llvm::sys::fs::exists(path) && !llvm::sys::fs::exists(value)) { std::string m_description; HEALTH_LOG_PRINTF("Nonexistent explicit module file %s", path.data()); return true; From d4589021f4abda12d82c8c8d5eafb6494c061f71 Mon Sep 17 00:00:00 2001 From: Dave Lee Date: Mon, 11 Mar 2024 15:49:59 -0700 Subject: [PATCH 4/9] Add test --- .../TypeSystem/Swift/SwiftASTContext.cpp | 2 +- .../implicit_fallback/Makefile | 4 +++ ...estSwiftExplicitModulesImplicitFallback.py | 29 +++++++++++++++++++ .../implicit_fallback/main.swift | 10 +++++++ 4 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile create mode 100644 lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py create mode 100644 lldb/test/API/lang/swift/explicit_modules/implicit_fallback/main.swift diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index 2cb9e1cd705ea..683d49a06c141 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -1614,7 +1614,7 @@ bool HasNonexistentExplicitModule(const std::vector &args) { // Check both path and value. This is to handle paths containing '='. if (!llvm::sys::fs::exists(path) && !llvm::sys::fs::exists(value)) { std::string m_description; - HEALTH_LOG_PRINTF("Nonexistent explicit module file %s", path.data()); + HEALTH_LOG_PRINTF("Nonexistent explicit module file %s", arg.data()); return true; } } diff --git a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile new file mode 100644 index 0000000000000..c5e32f304d138 --- /dev/null +++ b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile @@ -0,0 +1,4 @@ +SWIFT_SOURCES := main.swift +SWIFT_ENABLE_EXPLICIT_MODULES := YES +USE_PRIVATE_MODULE_CACHE := YES +include Makefile.rules diff --git a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py new file mode 100644 index 0000000000000..24f18d44bbb2f --- /dev/null +++ b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py @@ -0,0 +1,29 @@ +import shutil +import lldb +from lldbsuite.test.decorators import * +import lldbsuite.test.lldbtest as lldbtest +import lldbsuite.test.lldbutil as lldbutil + + +class TestCase(lldbtest.TestBase): + @swiftTest + def test_missing_explicit_modules(self): + """Test missing explicit Swift modules and fallback to implicit modules.""" + self.build() + + # This test verifies the case where explicit modules are missing. + # Remove explicit modules from their place in the module cache. + mod_cache = self.getBuildArtifact("private-module-cache") + shutil.rmtree(mod_cache) + + lldbutil.run_to_source_breakpoint( + self, "Set breakpoint here", lldb.SBFileSpec("main.swift") + ) + + log = self.getBuildArtifact("types.log") + self.runCmd(f"log enable lldb types -f '{log}'") + + self.expect("expression c", substrs=["hello implicit fallback"]) + + self.filecheck(f"platform shell cat {log}", __file__) + # CHECK: Nonexistent explicit module file diff --git a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/main.swift b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/main.swift new file mode 100644 index 0000000000000..1481f2c8298a9 --- /dev/null +++ b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/main.swift @@ -0,0 +1,10 @@ +class Class { + var msg : String = "hello implicit fallback" +} + +func main() { + let c = Class() + print(c) // Set breakpoint here +} + +main() From 610d19fba267150480d6180368f25338f70c4934 Mon Sep 17 00:00:00 2001 From: Dave Lee Date: Mon, 11 Mar 2024 15:51:48 -0700 Subject: [PATCH 5/9] Replace auto with std::string --- lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp index 683d49a06c141..1d47d73048a0d 100644 --- a/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp +++ b/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp @@ -1599,7 +1599,7 @@ void SwiftASTContext::AddExtraClangArgs(const std::vector &source, namespace { bool HasNonexistentExplicitModule(const std::vector &args) { - for (const auto &arg : args) { + for (const std::string &arg : args) { StringRef value = arg; if (!value.consume_front("-fmodule-file=")) continue; From f2f5d0b5ce7009f7e65d72a0f3eb26a01578b947 Mon Sep 17 00:00:00 2001 From: Dave Lee Date: Mon, 11 Mar 2024 16:24:20 -0700 Subject: [PATCH 6/9] Check that implicit modules are loaded --- .../TestSwiftExplicitModulesImplicitFallback.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py index 24f18d44bbb2f..77d78d749bacd 100644 --- a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py +++ b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py @@ -27,3 +27,4 @@ def test_missing_explicit_modules(self): self.filecheck(f"platform shell cat {log}", __file__) # CHECK: Nonexistent explicit module file + # CHECK: loaded module {{.*}} loaded: {{.*}}module-cache-lldb From 0829c9fb3b16d70f421a2e4b7d798612fc86ea6d Mon Sep 17 00:00:00 2001 From: Dave Lee Date: Tue, 12 Mar 2024 10:14:12 -0700 Subject: [PATCH 7/9] Remove check that isn't working in CI --- .../TestSwiftExplicitModulesImplicitFallback.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py index 77d78d749bacd..24f18d44bbb2f 100644 --- a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py +++ b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py @@ -27,4 +27,3 @@ def test_missing_explicit_modules(self): self.filecheck(f"platform shell cat {log}", __file__) # CHECK: Nonexistent explicit module file - # CHECK: loaded module {{.*}} loaded: {{.*}}module-cache-lldb From b7265d44c15f1189c5165af1b3c13ed220e73256 Mon Sep 17 00:00:00 2001 From: Dave Lee Date: Tue, 12 Mar 2024 14:16:59 -0700 Subject: [PATCH 8/9] Debug linux CI --- .../API/lang/swift/explicit_modules/implicit_fallback/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile index c5e32f304d138..0c77a2faac8ae 100644 --- a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile +++ b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile @@ -1,4 +1,5 @@ SWIFT_SOURCES := main.swift SWIFT_ENABLE_EXPLICIT_MODULES := YES USE_PRIVATE_MODULE_CACHE := YES +SWIFTFLAGS_EXTRAS := -v include Makefile.rules From da5163d971ef09fbc266f3adae54b8e0fbc167b8 Mon Sep 17 00:00:00 2001 From: Dave Lee Date: Fri, 15 Mar 2024 09:22:12 -0700 Subject: [PATCH 9/9] Disable test on Linux --- .../API/lang/swift/explicit_modules/implicit_fallback/Makefile | 1 - .../TestSwiftExplicitModulesImplicitFallback.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile index 0c77a2faac8ae..c5e32f304d138 100644 --- a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile +++ b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/Makefile @@ -1,5 +1,4 @@ SWIFT_SOURCES := main.swift SWIFT_ENABLE_EXPLICIT_MODULES := YES USE_PRIVATE_MODULE_CACHE := YES -SWIFTFLAGS_EXTRAS := -v include Makefile.rules diff --git a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py index 24f18d44bbb2f..86fc09ba8474c 100644 --- a/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py +++ b/lldb/test/API/lang/swift/explicit_modules/implicit_fallback/TestSwiftExplicitModulesImplicitFallback.py @@ -7,6 +7,7 @@ class TestCase(lldbtest.TestBase): @swiftTest + @skipIf(oslist=["linux"], bugnumber="rdar://124691219") def test_missing_explicit_modules(self): """Test missing explicit Swift modules and fallback to implicit modules.""" self.build()