sgraenitz marked an inline comment as done.
sgraenitz added inline comments.
================
Comment at: source/Utility/ConstString.cpp:123-126
+ assert((map.find(demangled) == map.end() || strlen(map[demangled]) == 0
||
+ map[demangled] == mangled_ccstr) &&
+ "The demangled string must have a unique counterpart or otherwise
"
+ "it must be empty");
----------------
friss wrote:
> In an asserts build, this is going to d a bunch of additional lookups. Can we
> move the assert after the try_emplace and basically assert that entry.second
> == nullptr || !strcmp(mangled_ccstr, entry.second).
>
> (It's unclear to me whether a pointer comparison is enough to test string
> equality in this case, do we know that everything passed to this function has
> been uniqued?)
> Can we move the assert after the try_emplace
Yes, right that's better.
> and basically assert that entry.second == nullptr || !strcmp(mangled_ccstr,
> entry.second)
I think it's worth allowing "overwrite empty string". There is code that does
`my_const_string.SetCString("")` to indicate failure, maybe other code does it
to indicate e.g. later..
> It's unclear to me whether a pointer comparison is enough
I think it's safe to keep it. The `ccstr` postfix is used all over to indicate
that the code assumes a pool string.
`Pool::GetConstCStringAndSetMangledCounterPart` is only used once in
`ConstString` and the `Pool` class is defined locally in the cpp.
https://reviews.llvm.org/D50536
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
