[Lldb-commits] [lldb] [lldb][Module] Remove feedback_stream parameter from LoadScriptingResources (PR #186787)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 16 05:15:58 PDT 2026
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/186787
I'm in the process of making `LoadScriptingResources` interactively ask a user whether to load a script. I'd like to turn the existing warning into the prompt. The simplest way to achieve this is to not print into a `feedback_stream` parameter, and instead create a prompt right there. This patch removes the `feedback_stream` parameter and emits a `ReportWarning` instead. If we get around to adding the prompt instead of the warning, those changes will be simpler to review. But even if we don't end up replacing the warning with a prompt, moving away from output parameters and towards more structured error reporting is a nice-to-have (e.g., the `warning` prefix is now colored, IDEs have more flexibility on how to present the warning, etc.).
For a command-line user nothing should change with this patch (apart from `warning:` being highlighted).
>From f54db71b16a28ea925855488ca724a56a506e0d1 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 16 Mar 2026 11:48:11 +0000
Subject: [PATCH] [lldb][Module] Remove feedback_stream parameter from
LoadScriptingResources
I'm in the process of making `LoadScriptingResources` interactively ask a user whether to load a script. I'd like to turn the existing warning into the prompt. The simplest way to achieve this is to not print into a `feedback_stream` parameter, and instead create a prompt right there. This patch removes the `feedback_stream` parameter and emits a `ReportWarning` instead. If we get around to adding the prompt instead of the warning, those changes will be simpler to review. But even if we don't end up replacing the warning with a prompt, moving away from output parameters and towards more structured error reporting is a nice-to-have (e.g., the `warning` prefix is now colored, IDEs have more flexibility on how to present the warning, etc.).
For a command-line user nothing should change with this patch (apart from `warning:` being highlighted).
---
lldb/include/lldb/Core/Module.h | 3 +-
lldb/include/lldb/Core/ModuleList.h | 1 -
lldb/include/lldb/Target/Target.h | 5 +--
lldb/source/Commands/CommandObjectTarget.cpp | 6 +--
lldb/source/Core/Debugger.cpp | 5 +--
lldb/source/Core/Module.cpp | 17 +++++---
lldb/source/Core/ModuleList.cpp | 4 +-
.../Platform/MacOSX/PlatformDarwin.cpp | 4 +-
lldb/source/Target/Target.cpp | 7 +---
.../dsym-python-script-name-warnings.test | 39 +++++++++++++++++++
.../unittests/Platform/PlatformDarwinTest.cpp | 8 ++--
11 files changed, 63 insertions(+), 36 deletions(-)
create mode 100644 lldb/test/Shell/Platform/AutoLoad/Darwin/dsym-python-script-name-warnings.test
diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index f46b3a13f4f18..ea875ac68cde2 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -510,8 +510,7 @@ class Module : public std::enable_shared_from_this<Module>,
/// \b true if it is, \b false otherwise.
bool IsLoadedInTarget(Target *target);
- bool LoadScriptingResourceInTarget(Target *target, Status &error,
- Stream &feedback_stream);
+ bool LoadScriptingResourceInTarget(Target *target, Status &error);
/// Get the number of compile units for this module.
///
diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h
index 740b986444a93..e147eeac61952 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -488,7 +488,6 @@ class ModuleList {
bool IsEmpty() const { return !GetSize(); }
bool LoadScriptingResourcesInTarget(Target *target, std::list<Status> &errors,
- Stream &feedback_stream,
bool continue_on_error = true);
static ModuleListProperties &GetGlobalModuleListProperties();
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 4f5b022765f9e..7907ed2f1a5f6 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1115,10 +1115,9 @@ class Target : public std::enable_shared_from_this<Target>,
LoadDependentFiles load_dependent_files = eLoadDependentsDefault);
bool LoadScriptingResources(std::list<Status> &errors,
- Stream &feedback_stream,
bool continue_on_error = true) {
- return m_images.LoadScriptingResourcesInTarget(
- this, errors, feedback_stream, continue_on_error);
+ return m_images.LoadScriptingResourcesInTarget(this, errors,
+ continue_on_error);
}
/// Get accessor for the images for this process.
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index d4b0b29390f92..e4f0679a7d884 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4402,9 +4402,7 @@ class CommandObjectTargetSymbolsAdd : public CommandObjectParsed {
// Make sure we load any scripting resources that may be embedded
// in the debug info files in case the platform supports that.
Status error;
- StreamString feedback_stream;
- module_sp->LoadScriptingResourceInTarget(target, error,
- feedback_stream);
+ module_sp->LoadScriptingResourceInTarget(target, error);
if (error.Fail() && error.AsCString())
result.AppendWarningWithFormat(
"unable to load scripting data for module %s - error "
@@ -4413,8 +4411,6 @@ class CommandObjectTargetSymbolsAdd : public CommandObjectParsed {
.GetFileNameStrippingExtension()
.GetCString(),
error.AsCString());
- else if (feedback_stream.GetSize())
- result.AppendWarning(feedback_stream.GetData());
flush = true;
result.SetStatus(eReturnStatusSuccessFinishResult);
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index d9cde68b7d5f0..fc099891443df 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -307,13 +307,10 @@ Status Debugger::SetPropertyValue(const ExecutionContext *exe_ctx,
if (target_sp->TargetProperties::GetLoadScriptFromSymbolFile() ==
eLoadScriptFromSymFileTrue) {
std::list<Status> errors;
- StreamString feedback_stream;
- if (!target_sp->LoadScriptingResources(errors, feedback_stream)) {
+ if (!target_sp->LoadScriptingResources(errors)) {
lldb::StreamUP s = GetAsyncErrorStream();
for (auto &error : errors)
s->Printf("%s\n", error.AsCString());
- if (feedback_stream.GetSize())
- s->PutCString(feedback_stream.GetString());
}
}
}
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 23921925a987b..eb84c566a1512 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1423,8 +1423,7 @@ bool Module::IsLoadedInTarget(Target *target) {
return false;
}
-bool Module::LoadScriptingResourceInTarget(Target *target, Status &error,
- Stream &feedback_stream) {
+bool Module::LoadScriptingResourceInTarget(Target *target, Status &error) {
if (!target) {
error = Status::FromErrorString("invalid destination Target");
return false;
@@ -1448,9 +1447,13 @@ bool Module::LoadScriptingResourceInTarget(Target *target, Status &error,
return false;
}
+ StreamString feedback_stream;
FileSpecList file_specs = platform_sp->LocateExecutableScriptingResources(
target, *this, feedback_stream);
+ if (!feedback_stream.Empty())
+ debugger.ReportWarning(feedback_stream.GetString().str(), debugger.GetID());
+
const uint32_t num_specs = file_specs.GetSize();
if (num_specs == 0)
return true;
@@ -1467,8 +1470,9 @@ bool Module::LoadScriptingResourceInTarget(Target *target, Status &error,
continue;
if (should_load == eLoadScriptFromSymFileWarn) {
- feedback_stream.Format(R"(
-warning: '{0}' contains a debug script. To run this script in this debug session:
+ debugger.ReportWarning(
+ llvm::formatv(
+ R"('{0}' contains a debug script. To run this script in this debug session:
command script import "{1}"
@@ -1476,8 +1480,9 @@ To run all discovered debug scripts in this session:
settings set target.load-script-from-symbol-file true
)",
- GetFileSpec().GetFileNameStrippingExtension(),
- scripting_fspec.GetPath());
+ GetFileSpec().GetFileNameStrippingExtension(),
+ scripting_fspec.GetPath()),
+ debugger.GetID());
return false;
}
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 898b0a2efb6a4..608d13c2a9fe3 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -1335,7 +1335,6 @@ bool ModuleList::RemoveSharedModuleIfOrphaned(const ModuleWP module_wp) {
bool ModuleList::LoadScriptingResourcesInTarget(Target *target,
std::list<Status> &errors,
- Stream &feedback_stream,
bool continue_on_error) {
if (!target)
return false;
@@ -1349,8 +1348,7 @@ bool ModuleList::LoadScriptingResourcesInTarget(Target *target,
for (auto module : tmp_module_list.ModulesNoLocking()) {
if (module) {
Status error;
- if (!module->LoadScriptingResourceInTarget(target, error,
- feedback_stream)) {
+ if (!module->LoadScriptingResourceInTarget(target, error)) {
if (error.Fail() && error.AsCString()) {
error = Status::FromErrorStringWithFormat(
"unable to load scripting data for "
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 36c9a8a26825e..039b4754fe0fc 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -287,7 +287,7 @@ FileSpecList PlatformDarwin::LocateExecutableScriptingResourcesFromDSYM(
: "contains reserved characters";
if (FileSystem::Instance().Exists(script_fspec))
feedback_stream.Format(
- "warning: the symbol file '{0}' contains a debug "
+ "the symbol file '{0}' contains a debug "
"script. However, its name"
" '{1}' {2} and as such cannot be loaded. LLDB will"
" load '{3}' instead. Consider removing the file with "
@@ -297,7 +297,7 @@ FileSpecList PlatformDarwin::LocateExecutableScriptingResourcesFromDSYM(
reason_for_complaint, path_string.GetString());
else
feedback_stream.Format(
- "warning: the symbol file '{0}' contains a debug "
+ "the symbol file '{0}' contains a debug "
"script. However, its name"
" {1} and as such cannot be loaded. If you intend"
" to have this script loaded, please rename '{2}' to "
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 9c8124a15333b..296a18f89dba2 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1545,9 +1545,7 @@ Module *Target::GetExecutableModulePointer() {
static void LoadScriptingResourceForModule(const ModuleSP &module_sp,
Target *target) {
Status error;
- StreamString feedback_stream;
- if (module_sp && !module_sp->LoadScriptingResourceInTarget(target, error,
- feedback_stream)) {
+ if (module_sp && !module_sp->LoadScriptingResourceInTarget(target, error)) {
if (error.AsCString())
target->GetDebugger().GetAsyncErrorStream()->Printf(
"unable to load scripting data for module %s - error reported was "
@@ -1555,9 +1553,6 @@ static void LoadScriptingResourceForModule(const ModuleSP &module_sp,
module_sp->GetFileSpec().GetFileNameStrippingExtension().GetCString(),
error.AsCString());
}
- if (feedback_stream.GetSize())
- target->GetDebugger().GetAsyncErrorStream()->Printf(
- "%s\n", feedback_stream.GetData());
}
void Target::ClearModules(bool delete_locations) {
diff --git a/lldb/test/Shell/Platform/AutoLoad/Darwin/dsym-python-script-name-warnings.test b/lldb/test/Shell/Platform/AutoLoad/Darwin/dsym-python-script-name-warnings.test
new file mode 100644
index 0000000000000..a5d5b8a4474e5
--- /dev/null
+++ b/lldb/test/Shell/Platform/AutoLoad/Darwin/dsym-python-script-name-warnings.test
@@ -0,0 +1,39 @@
+# REQUIRES: python, system-darwin
+
+# Tests that LLDB prints warning messages that occur while locating scripts in dSYMs.
+
+# RUN: split-file %s %t
+
+## Module name contains reserved characters but no script with a corrected
+## name exists.
+
+# RUN: %clang_host -g %t/main.c -o "%t/Test-Module.out"
+# RUN: mkdir -p "%t/Test-Module.out.dSYM/Contents/Resources/Python"
+# RUN: touch "%t/Test-Module.out.dSYM/Contents/Resources/Python/Test-Module.py"
+# RUN: %lldb -b \
+# RUN: -o 'target create "%t/Test-Module.out"' 2>&1 \
+# RUN: | FileCheck %s --check-prefix=CHECK-RENAME
+
+# CHECK-RENAME: warning: the symbol file {{.*}} contains reserved characters
+# CHECK-RENAME-SAME: If you intend to have this script loaded, please rename
+
+## Module name contains reserved characters but a script with a corrected
+## name does exists.
+
+# RUN: %clang_host -g %t/main.c -o "%t/Test-Module2.out"
+# RUN: mkdir -p "%t/Test-Module2.out.dSYM/Contents/Resources/Python"
+# RUN: touch "%t/Test-Module2.out.dSYM/Contents/Resources/Python/Test_Module2.py"
+# RUN: touch "%t/Test-Module2.out.dSYM/Contents/Resources/Python/Test-Module2.py"
+# RUN: %lldb -b \
+# RUN: -o 'target create "%t/Test-Module2.out"' 2>&1 \
+# RUN: | FileCheck %s --check-prefix=CHECK-REMOVE
+
+# CHECK-REMOVE: warning: the symbol file {{.*}} contains reserved characters
+# CHECK-REMOVE-SAME: Consider removing the file with the malformed name to eliminate this warning
+
+## Also confirm that the warning message about loading auto-loading scripts is printed
+## afterwards.
+# CHECK-REMOVE: warning: 'Test-Module2' contains a debug script. To run this script in this
+
+#--- main.c
+int main() { return 0; }
diff --git a/lldb/unittests/Platform/PlatformDarwinTest.cpp b/lldb/unittests/Platform/PlatformDarwinTest.cpp
index e6d1beb3a75e9..219f4dcddda71 100644
--- a/lldb/unittests/Platform/PlatformDarwinTest.cpp
+++ b/lldb/unittests/Platform/PlatformDarwinTest.cpp
@@ -354,7 +354,7 @@ TEST_F(PlatformDarwinLocateTest,
std::string fixed_script =
(m_tmp_dsym_dwarf_dir + "/../Python/_import.py").str();
std::string expected = llvm::formatv(
- "warning: the symbol file '{0}' contains a debug script. However, its "
+ "the symbol file '{0}' contains a debug script. However, its "
"name conflicts with a keyword and as such cannot be loaded. If you "
"intend to have this script loaded, please rename '{1}' to '{2}' and "
"retry.\n",
@@ -394,7 +394,7 @@ TEST_F(PlatformDarwinLocateTest,
std::string fixed_script =
(m_tmp_dsym_dwarf_dir + "/../Python/_import.py").str();
std::string expected = llvm::formatv(
- "warning: the symbol file '{0}' contains a debug script. However, its "
+ "the symbol file '{0}' contains a debug script. However, its "
"name '{1}' conflicts with a keyword and as such cannot be loaded. LLDB "
"will load '{2}' instead. Consider removing the file with the malformed "
"name to eliminate this warning.\n",
@@ -461,7 +461,7 @@ TEST_F(
std::string fixed_script =
(m_tmp_dsym_dwarf_dir + "/../Python/TestModule_1_1_1.py").str();
std::string expected = llvm::formatv(
- "warning: the symbol file '{0}' contains a debug script. However, its "
+ "the symbol file '{0}' contains a debug script. However, its "
"name contains reserved characters and as such cannot be loaded. If you "
"intend to have this script loaded, please rename '{1}' to '{2}' and "
"retry.\n",
@@ -502,7 +502,7 @@ TEST_F(
std::string fixed_script =
(m_tmp_dsym_dwarf_dir + "/../Python/TestModule_1_1_1.py").str();
std::string expected = llvm::formatv(
- "warning: the symbol file '{0}' contains a debug script. However, its "
+ "the symbol file '{0}' contains a debug script. However, its "
"name '{1}' contains reserved characters and as such cannot be loaded. "
"LLDB will load '{2}' instead. Consider removing the file with the "
"malformed name to eliminate this warning.\n",
More information about the lldb-commits
mailing list