[Lldb-commits] [PATCH] D124579: Make partial function name matching match on context boundaries
Jim Ingham via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Fri Apr 29 14:59:14 PDT 2022
jingham marked 6 inline comments as done.
jingham added inline comments.
================
Comment at: lldb/include/lldb/Target/Language.h:225
+ // symbol was really B::A::my_function. We want that to be
+ // a match. But we wouldn't want this to match AnotherB::A::my_function. The
+ // user is specifying a truncated path, not a truncated set of characters.
----------------
clayborg wrote:
> The comment should be adding something extra to "A" right? See suggested code change
Yes, the comment was wrong. I had a fancier example that was taking too many characters and I didn't delete enough when simplifying. But I'm not sure what code change you are referring to? I don't see how this is related to GetFunctionName.
================
Comment at: lldb/source/Core/Module.cpp:753-754
+ llvm::StringRef full_name = sc.GetFunctionName().GetStringRef();
+ if (full_name.empty() && sc.symbol)
+ full_name = sc.symbol->GetName().GetStringRef();
+ // We always keep unnamed symbols:
----------------
clayborg wrote:
> Remove this, SymbolContext::GetFunctionName() already returns the symbol name if there is no block with inlined function name or concrete function name.
Yeah, I swear I saw a case debugging through this where GetFunctionName was returning a null string but going to the Symbol worked, but reading through GetFunctionName I don't see how that could be and I can't reproduce the problem, so I must have been seeing some unrelated error.
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:293-309
+ // Incoming path has context but this method does not, no match.
+ if (m_context.empty())
+ return false;
+
+ // Make sure the incoming path's context is found in the method context:
+ size_t path_pos = m_context.rfind(context);
+ if (path_pos == llvm::StringRef::npos)
----------------
labath wrote:
> ```
> StringRef haystack = m_context;
> if (!haystack.consume_back(context))
> return false;
> if (haystack.empty() || !isalnum(haystack.back()))
> return true;
> return false;
> ```
Much nicer, thanks!
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:330
+
+// size_t from = 0;
+// llvm::StringRef demangled_ref = demangled.GetStringRef();
----------------
clayborg wrote:
> shafik wrote:
> > dead code?
> delete commented out code
Yup, that was the not very good (but much easier to implement) first try. It's gone...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124579/new/
https://reviews.llvm.org/D124579
More information about the lldb-commits
mailing list