wallace 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) {
----------------
teemperor wrote:
> 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.
The number of variables shouldn't be massive, so I'll change it to std::string
just for safety. I don't want to spend a lot of time debugging this if at some
point the assumption doensn't hold.
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
