[Lldb-commits] [PATCH] D114627: [lldb] add new overload for SymbolFile::FindTypes that accepts a scope

[Lasse Folger via Phabricator via lldb-commits]

lassefolger added inline comments.


================
Comment at: lldb/include/lldb/Symbol/SymbolFile.h:241
+  virtual void
+  FindTypes(ConstString name, ConstString scope,
+            const CompilerDeclContext &parent_decl_ctx, uint32_t max_matches,
----------------
JDevlieghere wrote:
> Does the scope need to be a ConstString? These strings are kept in memory forever and we should be mindful of their use. Could this a StringRef? 
I don't think it has to be a ConstString. I used a ConstString for consistency with `name`.
Let me change it to StringRef


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2456
+  }
+  std::string storage;
+
----------------
JDevlieghere wrote:
> Can this go into `if (has_scope) {`?
I don't mind putting it in there but thought reusing the same storage would avoid allocations.
Moving it into the function will lead to an allocation each time the function is called and the qualified name is too long for small string optimization.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114627/new/

https://reviews.llvm.org/D114627