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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
