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 wrote:
> 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()"
This is a good point. However, reading this code more, I wonder if the current
behavior is wrong -- in current behavior, after variable is located in parent
scope/object, it is checked to be expandable or not and is added into
expandable_variables list *before* new value assignment. I think the the
correct behavior should be only checking and add to expandable_variables list
*after* new value assignment, right?
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
