sgraenitz marked 2 inline comments as done.
sgraenitz added inline comments.
================
Comment at: include/lldb/Core/RichManglingInfo.h:83-84
+public:
+ RichManglingInfo *SetItaniumInfo();
+ RichManglingInfo *SetLegacyCxxParserInfo(const ConstString &mangled);
+
----------------
labath wrote:
> I find it odd that one of these functions is stateless
> (`SetLegacyCxxParserInfo` takes the string which initializes the object as an
> argument), while the other receives the state magically via the IPD accessor.
>
> Would it be possible to change it so that the other option is stateless too
> (`SetItaniumInfo(ConstString mangled)`)?
>
> We could change `RichManglingInfo` to return the full demangled name too, so
> that you have access to the demangled name via this API too (which /I think/
> is the reason why you created a separate `GetItaniumRichDemangleInfo`
> function).
> Would it be possible to change it so that the other option is stateless too
> (`SetItaniumInfo(ConstString mangled)`)?
I think that would be misleading for the reader. Symmetry is always nice to
indicate congruent behavior, but this is not the case here. The fallback C++
parser version mimics the previous behavior: demangle the name in step 1 and if
that succeeds, parse it with `CPlusPlusLanguage::MethodName` to decode its
properties. The IPD does that in a single step and so there is nothing to do in
`SetItaniumInfo()`. `SetLegacyCxxParserInfo()` on the other hand has to create
a `CPlusPlusLanguage::MethodName` from the mangled string.
> We could change `RichManglingInfo` to return the full demangled name too, so
> that you have access to the demangled name via this API too (which /I think/
> is the reason why you created a separate GetItaniumRichDemangleInfo function).
The interface wants to provide access to the name's encoded properties. That
doesn't really include the demangled "human readable" string representation
(it's also not needed in `Symtab::RegisterMangledNameEntry()`).
Of course `Symtab::InitNameIndexes()` will ask for `GetDemangledName()` just
after processing the mangled one. I think chances are good that for symbols
that are not filtered out anyway, we reached `DemangleWithRichManglingInfo()`
earlier and it will be in the string pool already (see Mangled.cpp:322).
Otherwise, yes it will be demangled in with a new IPD instance.
I think it's not bad to keep these cases as they are. I would propose to
investigate the performance of the current implementation in more detail and
think about the benefits before mixing them. Not sure at all, if it would bring
measurable gain.
================
Comment at: source/Core/RichManglingInfo.cpp:36-40
+RichManglingContext::SetLegacyCxxParserInfo(const ConstString &mangled) {
+ m_info.ResetProvider();
+ m_info.m_provider = RichManglingInfo::PluginCxxLanguage;
+ m_info.m_legacy_parser = new CPlusPlusLanguage::MethodName(mangled);
+ return &m_info;
----------------
labath wrote:
> Is this really the **mangled** name?
Yes, it is.
https://reviews.llvm.org/D50071
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
