aprantl added inline comments.
================ Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:152 + ConstString func_name = sym_ctx.GetFunctionName(); + if (func_name == ConstString(function_name) || + alternate_function_name.empty() || ---------------- JDevlieghere wrote: > teemperor wrote: > > JDevlieghere wrote: > > > I believe someone added an overload for comparing ConstStrings with > > > StringRefs. We shouldn't have to pay the price of creating one here just > > > for comparison. Same below. > > I really don't want a == overload for StringRef in `ConstString`. The > > *only* reason for using ConstString is using less memory for duplicate > > strings and being fast to compare against other ConstStrings. So if we add > > an overload for comparing against StringRef then code like this will look > > really innocent even though it essentially goes against the idea of > > ConstString. Either both string lists are using ConstString or neither does > > (which I would prefer). But having a list of normal strings and comparing > > it against ConstStrings usually means that the design is kinda flawed. Then > > we have both normal string comparisons but also the whole overhead of > > ConstString. > > > > Looking at this patch we already construct at one point a ConstString from > > the function name at one point, so that might as well be a ConstString in > > the first place. If we had an implicit comparison than the conversion here > > would look really innocent and no one would notice. > Hmm, you're totally right, I must be confusing this with something else then. > The *only* reason for using ConstString is using less memory for duplicate > strings and being fast to compare against other ConstStrings. Just for completeness, a huge use-case for ConstString in LLDB are as keys in DenseMaps and sorted vectors. This avoids redundantly string them in multiple StringMaps. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74252/new/ https://reviews.llvm.org/D74252 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
