yinghuitan added inline comments.
================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:531
+void Variables::Clear() {
+ locals.Clear();
----------------
clayborg wrote:
>
I am not sure. I like to call it Clear() in cause we will need to call it from
other scenario beyond process continue.
================
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:535
+ registers.Clear();
+ expandable_variables.clear();
+}
----------------
clayborg wrote:
> Do we want to set "next_temporary_var_ref" to VARREF_FIRST_VAR_IDX here too?
> Ok if we don't.
Good question. I guess we can reset it if wanted but not matter much though.
================
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;
----------------
clayborg wrote:
> rename to "temporary_variables" and add a comment
I kind of disagree with this. I think the keyword "expandable" is very
important for reading. It immediately tells reader that the map only contains
expandable variables instead of any other variables.
================
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;
+
----------------
clayborg wrote:
> rename to "permanent_variables" and add comment
The same as my comment above.
================
Comment at: lldb/tools/lldb-vscode/VSCode.h:114
+ /// Clear all scope variables and non-permanent expandable variables.
+ void Clear();
+};
----------------
clayborg wrote:
> clear sounds like you are clearing everything in the class. I would name this
> willContinue() like the others
I am not sure. I like to call it Clear() in cause we will need to call it from
other scenario beyond process continue. I do not think reader will be confused
by permanent map is not cleared because the name permanent imply that.
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
