[Lldb-commits] [PATCH] D49990: Use rich mangling information in Symtab::InitNameIndexes()

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Jul 30 09:06:42 PDT 2018


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





More information about the lldb-commits mailing list