Moved rich annotations flag into Disassembler options

UltimateForce21 · UltimateForce21 · commit cb0cd3abd5aa · 2025-08-11T19:28:16.000-04:00
This change refactors how the `--rich` flag is handled, based on code review feedback.

- Removed the `enable_rich_annotations` boolean from the API signatures of:
  - Disassembler::Disassemble(...)
  - Disassembler::PrintInstructions(...)
  - StackFrame::Disassemble(...)
- Added a new Disassembler::Option enum value: eOptionRichAnnotations.
- The `--rich` CLI flag now sets the new option bit in CommandObjectDisassemble::DoExecute:
    options |= Disassembler::eOptionRichAnnotations;
- Disassembler::PrintInstructions checks the bit to determine whether to enable rich annotations:
    const bool enable_rich = (options &amp; eOptionRichAnnotations) != 0;

The SB API remains unchanged and defaults to non-rich output.

Tested via the existing test using `disassemble --rich -f`.
diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h
@@ -398,6 +398,7 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>,
     eOptionMarkPCAddress =
         (1u << 3), // Mark the disassembly line the contains the PC
     eOptionShowControlFlowKind = (1u << 4),
+    eOptionRichAnnotations = (1u << 5),
   };
 
   enum HexImmediateStyle {
@@ -444,11 +445,10 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>,
                           const ExecutionContext &exe_ctx, const Address &start,
                           Limit limit, bool mixed_source_and_assembly,
                           uint32_t num_mixed_context_lines, uint32_t options,
-                          Stream &strm, bool enable_rich_annotations = false);
+                          Stream &strm);
 
   static bool Disassemble(Debugger &debugger, const ArchSpec &arch,
-                          StackFrame &frame, Stream &strm,
-                          bool enable_rich_annotations = false);
+                          StackFrame &frame, Stream &strm);
 
   // Constructors and Destructors
   Disassembler(const ArchSpec &arch, const char *flavor);
@@ -458,7 +458,7 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>,
                          const ExecutionContext &exe_ctx,
                          bool mixed_source_and_assembly,
                          uint32_t num_mixed_context_lines, uint32_t options,
-                         Stream &strm, bool enable_rich_annotations = false);
+                         Stream &strm);
 
   size_t ParseInstructions(Target &target, Address address, Limit limit,
                            Stream *error_strm_ptr,
diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h
@@ -321,7 +321,7 @@ class StackFrame : public ExecutionContextScope,
   ///
   /// \return
   ///    C string with the assembly instructions for this function.
-  const char *Disassemble(bool enable_rich_annotations = false);
+  const char *Disassemble();
 
   /// Print a description of this frame using the provided frame format.
   ///
diff --git a/lldb/source/Commands/CommandObjectDisassemble.cpp b/lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -533,6 +533,9 @@ void CommandObjectDisassemble::DoExecute(Args &command,
   if (m_options.raw)
     options |= Disassembler::eOptionRawOuput;
 
+  if (m_options.enable_rich_annotations)
+    options |= Disassembler::eOptionRichAnnotations;
+
   llvm::Expected<std::vector<AddressRange>> ranges =
       GetRangesForSelectedMode(result);
   if (!ranges) {
@@ -555,8 +558,7 @@ void CommandObjectDisassemble::DoExecute(Args &command,
             cpu_string, features_string, m_exe_ctx, cur_range.GetBaseAddress(),
             limit, m_options.show_mixed,
             m_options.show_mixed ? m_options.num_lines_context : 0, options,
-            result.GetOutputStream(),
-            /*enable_rich_annotations=*/m_options.enable_rich_annotations)) {
+            result.GetOutputStream())) {
       result.SetStatus(eReturnStatusSuccessFinishResult);
     } else {
       if (m_options.symbol_containing_addr != LLDB_INVALID_ADDRESS) {
diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp
@@ -169,12 +169,14 @@ Disassembler::DisassembleBytes(const ArchSpec &arch, const char *plugin_name,
   return disasm_sp;
 }
 
-bool Disassembler::Disassemble(
-    Debugger &debugger, const ArchSpec &arch, const char *plugin_name,
-    const char *flavor, const char *cpu, const char *features,
-    const ExecutionContext &exe_ctx, const Address &address, Limit limit,
-    bool mixed_source_and_assembly, uint32_t num_mixed_context_lines,
-    uint32_t options, Stream &strm, bool enable_rich_annotations) {
+bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch,
+                               const char *plugin_name, const char *flavor,
+                               const char *cpu, const char *features,
+                               const ExecutionContext &exe_ctx,
+                               const Address &address, Limit limit,
+                               bool mixed_source_and_assembly,
+                               uint32_t num_mixed_context_lines,
+                               uint32_t options, Stream &strm) {
   if (!exe_ctx.GetTargetPtr())
     return false;
 
@@ -189,10 +191,9 @@ bool Disassembler::Disassemble(
   if (bytes_disassembled == 0)
     return false;
 
-  disasm_sp->PrintInstructions(
-      debugger, arch, exe_ctx, mixed_source_and_assembly,
-      num_mixed_context_lines, options, strm,
-      /*enable_rich_annotations=*/enable_rich_annotations);
+  disasm_sp->PrintInstructions(debugger, arch, exe_ctx,
+                               mixed_source_and_assembly,
+                               num_mixed_context_lines, options, strm);
   return true;
 }
 
@@ -287,8 +288,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
                                      const ExecutionContext &exe_ctx,
                                      bool mixed_source_and_assembly,
                                      uint32_t num_mixed_context_lines,
-                                     uint32_t options, Stream &strm,
-                                     bool enable_rich_annotations) {
+                                     uint32_t options, Stream &strm) {
   // We got some things disassembled...
   size_t num_instructions_found = GetInstructionList().GetSize();
 
@@ -688,7 +688,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
                  show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr,
                  address_text_size);
 
-      if (enable_rich_annotations) {
+      if (options & eOptionRichAnnotations) {
         std::vector<std::string> annotations = annotate_variables(*inst);
         if (!annotations.empty()) {
           const size_t annotation_column = 100;
@@ -708,8 +708,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
 }
 
 bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch,
-                               StackFrame &frame, Stream &strm,
-                               bool enable_rich_annotations) {
+                               StackFrame &frame, Stream &strm) {
   constexpr const char *plugin_name = nullptr;
   constexpr const char *flavor = nullptr;
   constexpr const char *cpu = nullptr;
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
@@ -262,20 +262,16 @@ bool StackFrame::ChangePC(addr_t pc) {
   return true;
 }
 
-const char *StackFrame::Disassemble(bool enable_rich_annotations) {
+const char *StackFrame::Disassemble() {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
 
-  // Keep the existing cache only for the plain (non-rich) path.
-  if (!enable_rich_annotations) {
-    if (!m_disassembly.Empty())
-      return m_disassembly.GetData();
-  }
+  if (!m_disassembly.Empty())
+    return m_disassembly.GetData();
 
   ExecutionContext exe_ctx(shared_from_this());
   if (Target *target = exe_ctx.GetTargetPtr()) {
-    Disassembler::Disassemble(
-        target->GetDebugger(), target->GetArchitecture(), *this, m_disassembly,
-        /*enable_rich_annotations=*/enable_rich_annotations);
+    Disassembler::Disassemble(target->GetDebugger(), target->GetArchitecture(),
+                              *this, m_disassembly);
   }
 
   return m_disassembly.Empty() ? nullptr : m_disassembly.GetData();
