labath added a comment.
I haven't read through this in detail yet, but I think this is a good start!
The part I'm not sure about is whether the RichManglingInfo vs.
RichManglingSpec distinction brings any value. I mean, the lifetime of the
first is tied to the lifetime of the second, and the Spec class can only have
one active Info instance at any given moment. So you might as well just have
one class, pass that to `DemangleWithRichManglingInfo`, and then query the same
object when the call returns. The current interface with `createItaniumInfo` et
al. makes it seem like one could call it multiple times in sequence, stashing
the results, and then doing some post-processing on them.
I'll have to think about the C++::MethodName issue a bit more, but in general,
I don't think moving that class to a separate file is a too disruptive change.
If it means we don't have to mess with untyped pointers, then we should just do
it. (Ideally, I wouldn't want the common code to reference that plugin at all,
but that ship has already sailed, so I don't think this patch should be
predicated on fixing that.)
================
Comment at: include/lldb/Symbol/Symtab.h:81-83
+ // No move
+ RichManglingInfo(RichManglingInfo &&) = delete;
+ RichManglingInfo &operator=(RichManglingInfo &&) = delete;
----------------
This is implied by the deleted copy operations.
================
Comment at: source/Symbol/Symtab.cpp:271-289
+const char *RichManglingInfo::getFunctionBaseName() const {
+ switch (m_provider) {
+ case ItaniumPartialDemangler:
+ m_IPD_buf = m_IPD->getFunctionBaseName(m_IPD_buf, &m_IPD_size);
+ return m_IPD_buf;
+ case PluginCxxLanguage:
+ return get<CPlusPlusLanguage::MethodName>()->GetBasename().data();
----------------
Could these return StringRef instead of C strings?
================
Comment at: source/Symbol/Symtab.cpp:295
+namespace {
+bool lldb_skip_name(const char *mangled_cstr, Mangled::ManglingScheme scheme) {
+ switch (scheme) {
----------------
sgraenitz wrote:
> This uses a raw C-string instead of `llvm::StringRef` in order to achieve
> `O(1)` runtime.
If you changed the caller to use StringRef too (it seems possible at a first
glance) then this would still be O(1)
https://reviews.llvm.org/D49990
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
