[Lldb-commits] [lldb] [lldb] [disassembler] chore: update VariableAnnotator::Annotate to except only Instruction as param and drop module and target (PR #168276)
via lldb-commits
lldb-commits at lists.llvm.org
Sun Nov 16 07:58:17 PST 2025
https://github.com/n2h9 created https://github.com/llvm/llvm-project/pull/168276
## Description
This PR simplifies the `VariableAnnotator::Annotate` API by removing redundant parameters, making it easier to use
from future scripting APIs.
other minor changes (to follow lldb coding style):
- rename `annotate` to `Annotate`;
- rename Live_ and Current variables;
### Performance impact
- current version (in main) does one call to `GetModule()` per instruction;
- updated version makes two calls to `GetModule` per instruction.
(additional call to `GetModule` inside `Annotate` method.)
### Detailed
I have moved some refactoring from https://github.com/llvm/llvm-project/pull/165163 to make it easier to review and split to smaller parts, and this particular part could be treated as not related to structured API but as general enhancement of `VariableAnnotator`.
While working in this pr https://github.com/llvm/llvm-project/pull/165163 I have noticed that `VariableAnnotator::Annotate` accepts `instruction`, `module` and `target` as input params. Which could be simplified to depend only on the `instruction` object.
In particular:
- `target` is used to get architecture specification, which could be taken from `module`.
- `module` could be taken from `instruction`'s `Address` object.
Each variable annotation is rather tied to the instruction's execution point, then to the module or target as a whole. So, it looks like it will make api cleaner if it will depend only on `instruction` object.
## Testing
Unit tests are passing
<details>
<summary>screenshot</summary>
<img width="1268" height="849" alt="image" src="https://github.com/user-attachments/assets/417b4b3c-dea0-4348-b5ce-bf3deaa01d81" />
<details>
>From bfc643f5c6a302fd346bbe8dee65e0d7fcdcb482 Mon Sep 17 00:00:00 2001
From: Nikita B <n2h9z4 at gmail.com>
Date: Sun, 16 Nov 2025 14:32:52 +0100
Subject: [PATCH] [lldb] [disassembler] chore: update
VariableAnnotator::Annotate to except only Instruction as param and drop
module and target, rename Live_ and Current to follow lldb codding style
Signed-off-by: Nikita B <n2h9z4 at gmail.com>
---
lldb/include/lldb/Core/Disassembler.h | 10 +++---
lldb/source/Core/Disassembler.cpp | 44 +++++++++++++--------------
2 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h
index daa7b3d25f11b..5de314109b0cc 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -581,13 +581,13 @@ class VariableAnnotator {
};
// Live state from the previous instruction, keyed by Variable::GetID().
- llvm::DenseMap<lldb::user_id_t, VarState> Live_;
+ llvm::DenseMap<lldb::user_id_t, VarState> m_live_vars;
public:
- /// Compute annotation strings for a single instruction and update `Live_`.
- /// Returns only the events that should be printed *at this instruction*.
- std::vector<std::string> annotate(Instruction &inst, Target &target,
- const lldb::ModuleSP &module_sp);
+ /// Compute annotation strings for a single instruction and update
+ /// `m_live_vars`. Returns only the events that should be printed *at this
+ /// instruction*.
+ std::vector<std::string> Annotate(Instruction &inst);
};
} // namespace lldb_private
diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp
index f2ed1f7395346..431678f505f77 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -299,16 +299,16 @@ bool Disassembler::ElideMixedSourceAndDisassemblyLine(
// The goal is to give users helpful live variable hints alongside the
// disassembled instruction stream, similar to how debug information
// enhances source-level debugging.
-std::vector<std::string>
-VariableAnnotator::annotate(Instruction &inst, Target &target,
- const lldb::ModuleSP &module_sp) {
+std::vector<std::string> VariableAnnotator::Annotate(Instruction &inst) {
std::vector<std::string> events;
+ auto module_sp = inst.GetAddress().GetModule();
+
// If we lost module context, everything becomes <undef>.
if (!module_sp) {
- for (const auto &KV : Live_)
+ for (const auto &KV : m_live_vars)
events.emplace_back(llvm::formatv("{0} = <undef>", KV.second.name).str());
- Live_.clear();
+ m_live_vars.clear();
return events;
}
@@ -319,13 +319,13 @@ VariableAnnotator::annotate(Instruction &inst, Target &target,
if (!module_sp->ResolveSymbolContextForAddress(iaddr, mask, sc) ||
!sc.function) {
// No function context: everything dies here.
- for (const auto &KV : Live_)
+ for (const auto &KV : m_live_vars)
events.emplace_back(llvm::formatv("{0} = <undef>", KV.second.name).str());
- Live_.clear();
+ m_live_vars.clear();
return events;
}
- // Collect in-scope variables for this instruction into Current.
+ // Collect in-scope variables for this instruction into current_vars.
VariableList var_list;
// Innermost block containing iaddr.
if (Block *B = sc.block) {
@@ -341,7 +341,7 @@ VariableAnnotator::annotate(Instruction &inst, Target &target,
const lldb::addr_t func_file = sc.function->GetAddress().GetFileAddress();
// ABI from Target (pretty reg names if plugin exists). Safe to be null.
- lldb::ABISP abi_sp = ABI::FindPlugin(nullptr, target.GetArchitecture());
+ lldb::ABISP abi_sp = ABI::FindPlugin(nullptr, module_sp->GetArchitecture());
ABI *abi = abi_sp.get();
llvm::DIDumpOptions opts;
@@ -349,7 +349,7 @@ VariableAnnotator::annotate(Instruction &inst, Target &target,
// Prefer "register-only" output when we have an ABI.
opts.PrintRegisterOnly = static_cast<bool>(abi_sp);
- llvm::DenseMap<lldb::user_id_t, VarState> Current;
+ llvm::DenseMap<lldb::user_id_t, VarState> current_vars;
for (size_t i = 0, e = var_list.GetSize(); i != e; ++i) {
lldb::VariableSP v = var_list.GetVariableAtIndex(i);
@@ -376,16 +376,16 @@ VariableAnnotator::annotate(Instruction &inst, Target &target,
if (loc.empty())
continue;
- Current.try_emplace(v->GetID(),
- VarState{std::string(name), std::string(loc)});
+ current_vars.try_emplace(v->GetID(),
+ VarState{std::string(name), std::string(loc)});
}
- // Diff Live_ → Current.
+ // Diff m_live_vars → current_vars.
- // 1) Starts/changes: iterate Current and compare with Live_.
- for (const auto &KV : Current) {
- auto it = Live_.find(KV.first);
- if (it == Live_.end()) {
+ // 1) Starts/changes: iterate current_vars and compare with m_live_vars.
+ for (const auto &KV : current_vars) {
+ auto it = m_live_vars.find(KV.first);
+ if (it == m_live_vars.end()) {
// Newly live.
events.emplace_back(
llvm::formatv("{0} = {1}", KV.second.name, KV.second.last_loc).str());
@@ -396,14 +396,14 @@ VariableAnnotator::annotate(Instruction &inst, Target &target,
}
}
- // 2) Ends: anything that was live but is not in Current becomes <undef>.
- for (const auto &KV : Live_) {
- if (!Current.count(KV.first))
+ // 2) Ends: anything that was live but is not in current_vars becomes <undef>.
+ for (const auto &KV : m_live_vars) {
+ if (!current_vars.count(KV.first))
events.emplace_back(llvm::formatv("{0} = <undef>", KV.second.name).str());
}
// Commit new state.
- Live_ = std::move(Current);
+ m_live_vars = std::move(current_vars);
return events;
}
@@ -676,7 +676,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
address_text_size);
if ((options & eOptionVariableAnnotations) && target_sp) {
- auto annotations = annot.annotate(*inst, *target_sp, module_sp);
+ auto annotations = annot.Annotate(*inst);
if (!annotations.empty()) {
const size_t annotation_column = 100;
inst_line.FillLastLineToColumn(annotation_column, ' ');
More information about the lldb-commits
mailing list