teemperor added inline comments.
================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2934
+ // We first find out which variable names are duplicated
+ std::unordered_map<const char *, int> variable_name_counts;
+ for (auto i = start_idx; i < end_idx; ++i) {
----------------
clayborg wrote:
> We are getting a variable name via the public API which returns a "const char
> *". The only way this works is if the string comes from a ConstString
> otherwise there are lifetime issues. So this is and should be ok. The diff
> you are talking about was from internal LLDB code. For internal LLDB code we
> can enforce this by ensuring that people use ConstString, but externally
> through the API, we assume any "const char *" that comes out of it is uniqued
> and comes form a ConstString.
I don't think there is any promise that every `const char *` coming from the SB
API is generated from a ConstString. If there was such a promise a good chunk
of the SB API would already violate it.
That the `const char *` needs to have a reasonable life time (e.g., the same
life time as the SB API object that returns it) seems like a basic foundation
to have the API work at all. But promising that they are uniquified really
serves essentially no purpose? I sure hope that simply returning string
literals doesn't suddenly become an API violation (or that we need to forever
store every generated error message that we return from the SB API).
Also I think my example is spot on: An unverified assumption that doesn't hold
true. And once we change the underlying code someone has to spend a week
tracking down bogus string comparisons in the year 2021.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99989/new/
https://reviews.llvm.org/D99989
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
