jingham added a comment. In D118814#3297198 <https://reviews.llvm.org/D118814#3297198>, @JDevlieghere wrote:
> In D118814#3297075 <https://reviews.llvm.org/D118814#3297075>, @jingham wrote: > >> In D118814#3296008 <https://reviews.llvm.org/D118814#3296008>, @labath wrote: >> >>> This seems fine, though it's not clear to me what is the effect of this >>> patch in terms of functionality. Does the "side-effect" mentioned by Jim >>> still apply here, or is this NFC now? Either is probably fine, but I'd like >>> to understand what is going on. It seems like it should be NFC, but does >>> that mean that the demangling (and the cpu/memory cost) is delayed until >>> the first operation which requests it (such as matching a breakpoint by the >>> full demangled name) ? >> >> I haven't gone back to read our lookups in detail, but I certainly hope that >> the first time we see a breakpoint on a symbol name we don't recognize, we >> wouldn't go demangling every symbol name in the system. We really try to >> keep mistypings from cascading into "unpack the entire world" events. > > Yes, this does break the ability to set breakpoints on full demangled names. > Based on the code and the comments, it really looks like it was always the > intention to avoid demangling the whole name, but then (accidentally?) made > it work by storing it in the ConstString. The continue on line 333 is what > prevents us from indexing the full name. Before this patch, > `GetDemangledName` would return the cached full demanged name, which now > isn't cached and would have to be computed on demand (effectively defeating > the purpose of this patch and making things slower). We really should come up with a good story for symbol lookup on function names that include arguments, but "match the exact output of the demangler" was never a good story. And I don't think we need pre-demangled names to do it right, rather we should pull the method name out of the user specification, find the matches to that - which goes quickly 'cause it uses the name chopper indices - then winnow down the matches. Since this is just for function name matches, we could even do smart stuff like (this is spitballing, not a proposed design): (lldb) break set -n foo::bar::bar(int, *) meaning, the first parameter has to be an int, I don't care about the others, etc. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118814/new/ https://reviews.llvm.org/D118814 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
