Skip to content

[lldb] Remove -d(ebug) mode from the lldb driver #83330

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 1 commit into from
Feb 28, 2024

Conversation

JDevlieghere
Copy link
Member

The -d(ebug) option broke 5 years ago when I migrated the driver to libOption. Since then, we were never check if the option is set. We were incorrectly toggling the internal variable (m_debug_mode) based on OPT_no_use_colors instead.

Given that the functionality doesn't seem particularly useful and nobody noticed it has been broken for 5 years, I'm just removing the flag.

The -d(ebug) option broke 5 years ago when I migrated the driver to
libOption. Since then, we were never check if the option is set. We were
incorrectly toggling the internal variable (m_debug_mode) based on
OPT_no_use_colors instead.

Given that the functionality doesn't seem particularly useful and nobody
noticed it has been broken for 5 years, I'm just removing the flag.
@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

The -d(ebug) option broke 5 years ago when I migrated the driver to libOption. Since then, we were never check if the option is set. We were incorrectly toggling the internal variable (m_debug_mode) based on OPT_no_use_colors instead.

Given that the functionality doesn't seem particularly useful and nobody noticed it has been broken for 5 years, I'm just removing the flag.


Full diff: https://github.com/llvm/llvm-project/pull/83330.diff

3 Files Affected:

  • (modified) lldb/test/Shell/Driver/TestHelp.test (-2)
  • (modified) lldb/tools/driver/Driver.cpp (-15)
  • (modified) lldb/tools/driver/Driver.h (-1)
diff --git a/lldb/test/Shell/Driver/TestHelp.test b/lldb/test/Shell/Driver/TestHelp.test
index 0f73fdf0374fdd..2521b31a618836 100644
--- a/lldb/test/Shell/Driver/TestHelp.test
+++ b/lldb/test/Shell/Driver/TestHelp.test
@@ -37,8 +37,6 @@ CHECK: --arch
 CHECK: -a
 CHECK: --core
 CHECK: -c
-CHECK: --debug
-CHECK: -d
 CHECK: --editor
 CHECK: -e
 CHECK: --file
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index c63ff0ff597e5c..9286abb27e1332 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -188,7 +188,6 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
   if (args.hasArg(OPT_no_use_colors)) {
     m_debugger.SetUseColor(false);
     WithColor::setAutoDetectFunction(disable_color);
-    m_option_data.m_debug_mode = true;
   }
 
   if (args.hasArg(OPT_version)) {
@@ -455,16 +454,7 @@ int Driver::MainLoop() {
   // Process lldbinit files before handling any options from the command line.
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInGlobalDirectory(result);
-  if (m_option_data.m_debug_mode) {
-    result.PutError(m_debugger.GetErrorFile());
-    result.PutOutput(m_debugger.GetOutputFile());
-  }
-
   sb_interpreter.SourceInitFileInHomeDirectory(result, m_option_data.m_repl);
-  if (m_option_data.m_debug_mode) {
-    result.PutError(m_debugger.GetErrorFile());
-    result.PutOutput(m_debugger.GetOutputFile());
-  }
 
   // Source the local .lldbinit file if it exists and we're allowed to source.
   // Here we want to always print the return object because it contains the
@@ -536,11 +526,6 @@ int Driver::MainLoop() {
                             "or -s) are ignored in REPL mode.\n";
   }
 
-  if (m_option_data.m_debug_mode) {
-    result.PutError(m_debugger.GetErrorFile());
-    result.PutOutput(m_debugger.GetOutputFile());
-  }
-
   const bool handle_events = true;
   const bool spawn_thread = false;
 
diff --git a/lldb/tools/driver/Driver.h b/lldb/tools/driver/Driver.h
index d5779b3c2c91b5..83e0d8a41cfdb1 100644
--- a/lldb/tools/driver/Driver.h
+++ b/lldb/tools/driver/Driver.h
@@ -75,7 +75,6 @@ class Driver : public lldb::SBBroadcaster {
     std::vector<InitialCmdEntry> m_after_file_commands;
     std::vector<InitialCmdEntry> m_after_crash_commands;
 
-    bool m_debug_mode = false;
     bool m_source_quietly = false;
     bool m_print_version = false;
     bool m_print_python_path = false;

Copy link
Member

@medismailben medismailben left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@bulbazord bulbazord left a comment

Choose a reason for hiding this comment

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

:)

@JDevlieghere JDevlieghere merged commit d3173f4 into llvm:main Feb 28, 2024
@JDevlieghere JDevlieghere deleted the no-debug branch February 28, 2024 23:23
mylai-mtk pushed a commit to mylai-mtk/llvm-project that referenced this pull request Jul 12, 2024
The -d(ebug) option broke 5 years ago when I migrated the driver to
libOption. Since then, we were never check if the option is set. We were
incorrectly toggling the internal variable (m_debug_mode) based on
OPT_no_use_colors instead.

Given that the functionality doesn't seem particularly useful and nobody
noticed it has been broken for 5 years, I'm just removing the flag.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants