clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:531
+void Variables::Clear() {
+ locals.Clear();
----------------
================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:535
+ registers.Clear();
+ expandable_variables.clear();
+}
----------------
Do we want to set "next_temporary_var_ref" to VARREF_FIRST_VAR_IDX here too? Ok
if we don't.
================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:538
+
+int64_t Variables::GetNewVariableRefence(bool is_permanent) {
+ if (is_permanent)
----------------
rename
================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:549
+
+lldb::SBValue Variables::GetVariableFromVariableReference(int64_t var_ref) {
+ lldb::SBValue variable;
----------------
rename to "GetVariable" to help with 80 column limit and make const;
================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:550-555
+ lldb::SBValue variable;
+ if (IsPermanentVariableReference(var_ref))
+ variable = expandable_permanent_variables.find(var_ref)->second;
+ else
+ variable = expandable_variables.find(var_ref)->second;
+ return variable;
----------------
We need to guard against the variable reference not existing in the map to make
sure we never crash:
================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:558
+
+int64_t Variables::InsertNewExpandableVariable(lldb::SBValue
expandableVariable,
+ bool is_permanent) {
----------------
var name for LLDB and rename to shorter name.
================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:560
+ bool is_permanent) {
+ auto newVariablesReferences = GetNewVariableRefence(is_permanent);
+ if (is_permanent)
----------------
rename newVariablesReferences to "var_ref"
================
Comment at: lldb/tools/lldb-vscode/VSCode.h:94
+ int64_t next_permanent_var_ref{VARREF_FIRST_VAR_IDX};
+ llvm::DenseMap<int64_t, lldb::SBValue> expandable_variables;
+ llvm::DenseMap<int64_t, lldb::SBValue> expandable_permanent_variables;
----------------
rename to "temporary_variables" and add a comment
================
Comment at: lldb/tools/lldb-vscode/VSCode.h:95
+ llvm::DenseMap<int64_t, lldb::SBValue> expandable_variables;
+ llvm::DenseMap<int64_t, lldb::SBValue> expandable_permanent_variables;
+
----------------
rename to "permanent_variables" and add comment
================
Comment at: lldb/tools/lldb-vscode/VSCode.h:99
+ /// expandable_permanent_variables.
+ bool IsPermanentVariableReference(int64_t variableReference);
+
----------------
lldb variable name, and make static since it doesn't require any ivar access
================
Comment at: lldb/tools/lldb-vscode/VSCode.h:110
+ /// Insert a new \p expandableVariable.
+ int64_t InsertNewExpandableVariable(lldb::SBValue expandableVariable,
+ bool is_permanent);
----------------
Fix LLDB variable name
================
Comment at: lldb/tools/lldb-vscode/VSCode.h:114
+ /// Clear all scope variables and non-permanent expandable variables.
+ void Clear();
+};
----------------
clear sounds like you are clearing everything in the class. I would name this
willContinue() like the others
================
Comment at: lldb/tools/lldb-vscode/VSCode.h:238
+ /// Debuggee will continue from stopped state.
+ void willContinue() { variables.Clear(); }
+
----------------
Correct camel case for LLDB function names
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105166/new/
https://reviews.llvm.org/D105166
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
