[Lldb-commits] [lldb] [lldb][[DWARFDeclContext] Add function to extract qualified names as vector (PR #77349)

Felipe de Azevedo Piovezan via lldb-commits lldb-commits at lists.llvm.org
Tue Jan 9 11:55:26 PST 2024


================
@@ -68,6 +68,11 @@ class DWARFDeclContext {
 
   const char *GetQualifiedName() const;
 
+  /// Returns a vector of string, one string per entry in the fully qualified
+  /// name. For example, for the DeclContext `A::B::C`, this methods returns
+  /// `{"C", "B", "A"}`
+  llvm::SmallVector<llvm::StringRef> GetQualifiedNameAsVector() const;
----------------
felipepiovezan wrote:

Out of curiosity, why is a pair of name + tag called a "Compiler Context"?

> We could make a using directive for std::vector<CompilerContext> so we don't have to mention the std::vector all of the time :

A DWARFDeclContext has a couple of functions that are used elsewhere, so we can't replace it with a vector 1:1.  We'd probably still need an abstraction or a few free functions. 
Also [personal opinion] it is better to have `std::vector` if the abstraction _is_ just vector.

> comparing many matching entries becomes a pointer compare for the string instead of actual string comparisons.

This is assuming the thing that is compared against is _also_ a ConstString, which is not necessarily the case. 
This is probably a much longer discussion, but every time I've seen the string comparison argument in favor of using ConstString, I was never able to prove it has any measurable difference. It's obviously hard to prove a negative, but I have notes on experiments where I tried to show they make any difference inside FileSpecs, and could not prove that to be the case. I even made the StringRef::equals function extra slow (no-opt + no length shorcut) and still could not see any performance impact.


----

Getting back to the original discussion: if we feel strongly that adding this method is akin to more duplication, I don't mind closing this patch until we can do something about DWARFDeclContext and possibly remove it. 
In the meantime, the follow up patches to this can instead operate on `DWARFDeclContext::operator[]` + `Entry.Name` without too much extra burden

https://github.com/llvm/llvm-project/pull/77349


More information about the lldb-commits mailing list