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 wrote:
> We can't just make up a variable reference here, it needs to match the
> variable reference for "curr_variable" when it was first inserted. It used to
> be set to "i" before since that _was_ the actualy variable reference of the
> item we are changing the value of. It is fine for this to be 0 if we are
> setting say a local variable that is a "int x" since it has no variable
> reference, but we do need the variable reference to match the original.
>
> So, we need to track the ID given to each local, global, or register variable
> and map it to a variable reference.
>
> I am not really sure why the result of this needs to return the new variable
> reference, or how it is used in the IDE.
The quicker way to fix this without having to track a map of lldb::SBValue ->
var_ref would be to always insert it again into our temp lists:
```
newVariablesReference = g_vsc.variables.InsertExpandableVariable(variable,
false);
```
We would end up with multiple entries for this variable, but that should be ok
as they will get cleared out the next time we resume/stop with the Clear()
method...
================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2812
+ g_vsc.variables.IsPermanentVariableReference(variablesReference);
+ g_vsc.variables.InserExpandableVariable(variable, is_permanent);
}
----------------
clayborg wrote:
> Rename to "Insert(...)" and you must use the variable reference that was
> returned.
>
> That being said, the old code was incorrect as it was appending the same
> value again in the the "g_vsc.variables" list, but it didn't need to. It
> should have just been returning the old index of the for "variable" in the
> g_vsc.variables" list. So we will need a way to get the original variable
> reference for any "SBValue" back from the "g_vsc.variables" class. One idea
> is to rely on the following SBValue method:
> ```
> lldb::user_id_t SBValue::GetID();
> ```
> And when we call "g_vsc.variables.InserExpandableVariable(variable,
> is_permanent)" we have that method keep a map of "lldb::user_id_t ->
> var_ref".
Thinking about this some more, we should probably just go with the code fix
suggestion I made above. This might insert the same "variable" into the lists
again, but this only happens when we set a variable value, so this won't be too
often. Another point is people don't often edit top level structures, they edit
variables that have values. So if you have a "Point pt" variable, you won't
usually try to edit top level "pt" value, you would edit the "pt.x" or "pt.y"
values. If you do try to edit the top level structure value, it will probably
return an error. So it is doubtful that this will even cause many problems
since most items users will edit won't return "true" for a call to
"variable.MightHaveChildren()"
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
