JDevlieghere 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() || ---------------- 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. 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
