Michael137 added inline comments.
================ Comment at: lldb/source/Target/Target.cpp:207 + // E.g., this could happen on rebuild & relaunch + // of the debugee. + m_scratch_type_system_map.Clear(); ---------------- Michael137 wrote: > aprantl wrote: > > Not opposed to this, but this is leaking an implementation detail of > > TypeSystemClang into Target. Just out of curiosity — would if be feasible > > to use weak_ptr in DeclOrigin or is that defined inside of Clang and can't > > be changed? > I agree with your point about leaking TypeSystem implementation details into > Process lifecycle. I briefly considered converting `DeclOrigin`s to hold weak > pointers but unfortunately the pointers are handed out by `clang::Decl`. > Though thinking about it, `clang::ASTContext`s are refcounted (via > `llvm::RefCountedBase`). So perhaps we could keep them alive that way. Though > I’m concerned that we then commit to supporting multiple definitions of a > type in the scratch AST and never really properly clearing the `DeclOrigin` > structures. But I’ll play around with the idea Hmm this becomes a bit awkward because `TypeSystemClang` owns the backing ASTContext uniquely: `std::unique_ptr<clang::ASTContext> m_ast_up`. So we can't bump the refcount unless we somehow prevent the `m_ast_up` deleter from kicking in when the backing `lldb_private::Module` gets destroyed. Stepping back a little, essentially what we want is to: 1. Either, accept the fact that the `OriginMap` can contain stale pointers and provide a mechanism to detect this (and clear them out of the map) 2. OR, never allow a situation where we have stale pointers in this map. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138724/new/ https://reviews.llvm.org/D138724 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
