labath accepted this revision.
labath added a comment.
Some more comments inline, but I think this is essentially as good as we can
make it.
================
Comment at: lldb/include/lldb/Symbol/Type.h:300
- TypeSystem *GetTypeSystem(bool prefer_dynamic);
+ CompilerType::TypeSystemSPWrapper GetTypeSystem(bool prefer_dynamic);
----------------
Maybe the wrapper type should just be its own top-level entity? Or
`TypeSystem::SPWrapper`?
================
Comment at: lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp:193-194
+ if (!m_type_system_sp) {
+ m_type_system = new TypeSystemClang("siginfo", triple);
+ m_type_system_sp.reset(m_type_system);
+ }
----------------
Remove `m_type_system`, change `m_type_system_sp` to
`std::shared_ptr<TypeSystemClang>`
================
Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:316-321
+ std::lock_guard<std::mutex> guard(m_mutex);
+ if (!m_type_system_sp) {
+ m_type_system = new TypeSystemClang("siginfo", triple);
+ m_type_system_sp.reset(m_type_system);
+ }
+ }
----------------
Same here (but also remove the `_wp` variable).
================
Comment at: lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp:211
+ std::lock_guard<std::mutex> guard(m_mutex);
+ if (!m_type_system_sp) {
+ m_type_system = new TypeSystemClang("siginfo", triple);
----------------
and here
================
Comment at: lldb/source/Symbol/TypeSystem.cpp:278-279
m_map[language] = type_system_sp;
- if (type_system_sp.get())
- return *type_system_sp.get();
+ if (type_system_sp) {
+ assert(!type_system_sp->weak_from_this().expired());
+ return type_system_sp;
----------------
With the current implementation, I don't think these can ever fire.
================
Comment at: lldb/source/Target/Target.cpp:2461
+ Language::GetNameForLanguageType(language) +
+ llvm::StringRef("is no longer live"),
+ llvm::inconvertibleErrorCode());
----------------
Maybe this error needs some updating? (also, missing spaces -- the language
will get glued to the error msg)
================
Comment at: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h:48
+/// Simulates a Clang type system owned by a TypeSystemMap.
+class TypeSystemClangHolder {
+ TypeSystemClang *m_ast;
----------------
Can this be replaced by a `shared_ptr<TypeSystemClang>` now?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136650/new/
https://reviews.llvm.org/D136650
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
