[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: lldb/include/lldb/Symbol/CompilerType.h:58
+    template <class TypeSystemType> TypeSystemType *dyn_cast_or_null() {
+      return llvm::dyn_cast_or_null<TypeSystemType>(m_typesystem_sp.get());
+    }
----------------
labath wrote:
> Maybe do something like this instead:
> ```
> template <class TypeSystemType> std::shared_ptr<TypeSystemType> dyn_cast_or_null() {
>   if (llvm::isa<TypeSystemType>(m_typesystem_sp.get()))
>     return std::shared_ptr<TypeSystemType>(m_typesystem_sp, llvm::cast<TypeSystemType>(m_typesystem_sp.get()));
>   return nullptr;
> }
> ```
> 
> That said, `llvm::dyn_cast` supports custom casts for smart pointers (it already has them for std::unique_ptr), so it should be possible to completely avoid this wrapper class by implementing this logic inside llvm.
> Although, then we'd have to answer awkward questions like "why are you using a shared_ptr in the first place".
I think that's a good idea. Based on the assumption that we are going to rework the ownership model in LLDB to move away from shared pointers towards contexts at some point in the future, I'm not going to extend casting support in Support now.


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

https://reviews.llvm.org/D136650