[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

Fri Jan 13 16:00:21 PST 2023

clayborg added a comment.

New changes looks great, thanks for tackling this. Just a few comments that need to be updated and this will be good to go as long as the test suite passes just fine!

In D141318#4052934 <https://reviews.llvm.org/D141318#4052934>, @augusto2112 wrote:

> @clayborg I went with your suggestion and made Type's constructor private, and any new Type created is automatically added to the TypeList. I think later on we should make TypeList keep a vector of unique pointers instead of shared ones.

If we did switch the unique pointers, then we would always need to hand out "Type *" and this could cause crashes and use after free errors. So I would hold off on that change.

================
Comment at: lldb/include/lldb/Symbol/SymbolFile.h:414
   virtual void SetDebugInfoHadFrameVariableErrors() = 0;

+  virtual lldb::TypeSP
----------------
Just add a comment here saying something lie "This function is used to create types that belong to a SymbolFile. The symbol file will own a strong reference to the type in an internal type list."

================
Comment at: lldb/include/lldb/Symbol/Type.h:229-230
+private:
+  /// Only allow Symbol File to create types, as they should own them by keeping
+  /// them in their TypeList.
+  friend class lldb_private::SymbolFileCommon;
----------------
add a \see SymbolFileCommon::MakeType() reference in the header documentation here so users will know what function to use if the get a compile error.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141318