This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9139acb85a4: Fix expression evaluation result expansion in
yinghuitan added inline comments.
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1790
g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.WillContinue();
thread.StepOver();
clayborg wrote:
> Remove and handle in "eStateStopped" or "eStateRunning" as
yinghuitan updated this revision to Diff 361815.
yinghuitan marked 4 inline comments as done.
yinghuitan added a comment.
Fix the issue in setVariable request.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105166/new/
yinghuitan added inline comments.
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2812
+g_vsc.variables.IsPermanentVariableReference(variablesReference);
+g_vsc.variables.InserExpandableVariable(variable, is_permanent);
}
clayborg
clayborg added inline comments.
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2784
if (curr_variable.MightHaveChildren())
- newVariablesReference = i;
+ newVariablesReference = g_vsc.variables.GetNewVariableRefence(true);
break;
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
So there are issues with the setVariable and the variable reference it is
returning. See inlined comments.
Comment at: lldb/tools/lldb-vscode/VSCode.cpp:540
+
yinghuitan added inline comments.
Comment at: lldb/tools/lldb-vscode/VSCode.h:83
+struct Variables {
+ // Bit mask to tell if a variableReference is inside
wallace wrote:
> yinghuitan wrote:
> > wallace wrote:
> > > Move it to a new file and add a global
yinghuitan added a comment.
Debug console and VScode UI synchronization is a much bigger topic than
instruction level stepping so I do not plan to fix in this patch. For example,
if user change value via lldb comamnd, the watch/locals won't change. If user
switch stack frames/threads, it won't
yinghuitan updated this revision to Diff 360340.
yinghuitan added a comment.
Last push failed to include the changes due to git issue. Push including the
real changes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105166/new/
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.
There are a lot of comments from your previous diff that were not addressed. I
reviewed up to a certain point but fix the remaining ones anyway. About
documentation, try to be
yinghuitan added inline comments.
Comment at:
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py:386-389
+var_ref = permanent_expr_varref_dict[expandable_expression['name']]
+response = self.vscode.request_variables(var_ref)
+
yinghuitan updated this revision to Diff 360281.
yinghuitan marked 33 inline comments as done.
yinghuitan added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105166/new/
https://reviews.llvm.org/D105166
Files:
wallace requested changes to this revision.
wallace added inline comments.
Comment at:
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py:292
+Tests the evaluated expression expands successfully after "scopes"
packets
+and permanent
+
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
Comment at: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py:53
if varRef != 0 and varref_dict is not None:
-
yinghuitan updated this revision to Diff 355942.
yinghuitan marked 13 inline comments as done.
yinghuitan added a comment.
Address review comments and add testcase.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105166/new/
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
clayborg added a comment.
We will need to add tests for this as well to ensure it doesn't regress. Tests
should be added to:
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
I would add a new test function into the TestVSCode_variables class:
@skipIfWindows
@skipIfRemote
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:
yinghuitan updated this revision to Diff 355437.
yinghuitan added a comment.
rebase to latest master
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105166/new/
https://reviews.llvm.org/D105166
Files:
lldb/tools/lldb-vscode/VSCode.cpp
yinghuitan updated this revision to Diff 355436.
yinghuitan marked an inline comment as done.
yinghuitan added a comment.
Rerun linter
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105166/new/
https://reviews.llvm.org/D105166
Files:
yinghuitan updated this revision to Diff 355435.
yinghuitan marked 37 inline comments as done.
yinghuitan added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105166/new/
https://reviews.llvm.org/D105166
Files:
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Mostly LLDB and LLVM coding guideline conventions that need to be followed.
Many comments and code suggestions. But there are some code changes like having
unique IDs for temp
yinghuitan created this revision.
yinghuitan added reviewers: clayborg, wallace.
Herald added a subscriber: jfb.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
VScode now sends a "scopes" DAP request immediately after any
23 matches
Mail list logo
