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

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Aug 8 02:53:20 PDT 2018


labath added inline comments.


================
Comment at: source/Core/RichManglingContext.cpp:133
+  case PluginCxxLanguage:
+    m_cxx_method_str = ConstString(
+        get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)->GetBasename());
----------------
sgraenitz wrote:
> labath wrote:
> > I thought we were going to get rid of this ConstString? The C++ plugin is able to provide the same (in fact, even stronger) lifetime guarantees as the IPD, so we could just have a single method that always returns a StringRef. (If you really want to, you can also have a ConstString-returning helper function, but it could be implemented the same way as it is now for IPD, so that both versions have the same runtime complexity.
> > 
> > I'd suggest having just a m_buffer StringRef object, which you set both here and in processIPDStrResult. Then `GetBufferRef()` can just return that object without any branching involved.
> Yes, I can change the ConstString thing here.
> 
> > I'd suggest having just a m_buffer StringRef object
> You mean a single buffer for both, IPD and C++ method parser plugin? Actually I don't like to mix that. In case of IPD the buffer is owned by the context. Otherwise it's owned by the C++ method parser plugin. I could manage that in `FromItaniumName` and `FromCxxMethodName`, but it will make everything more complicated. Top prio for me is to avoid reallocation of the relatively big initial `m_IPD_buf` and I think the cleanest way to do this is the RAII-style that's used already.
Well, kind of. I guess I shouldn't have said /just/ an m_buffer object. For the itanium case, this buffer would be /in addition/ to the existing m_PID_XXX members.

The operating invariant would be that m_buffer is a non-owning reference to the string which was set by the last `ParseXXX` call. So, for the CPlusPlus case it means you would here to something like `m_buffer = get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)->GetBasename();`. And in the itanium case, you would set `m_buffer = StringRef(m_IPD_buf, m_IPD_str_size)`, after you finish all other work.

I don't think that should affect any of the work you do already, it should just make the GetBufferRef function simpler (and faster).


https://reviews.llvm.org/D50071





More information about the lldb-commits mailing list