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();
----------------
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
================
Comment at: lldb/source/Target/Target.cpp:208
+ // of the debugee.
+ m_scratch_type_system_map.Clear();
m_process_sp.reset();
----------------
kastiglione wrote:
> Do we have some place in the life-cycle where we can perform this only if the
> target has changed? Ideally this would happen when the binary has a different
> timestamp, or for mach-o a different UUID.
There is `DidUnloadModules` which gets notified when an lldb_private::Module
gets unloaded (e.g., on rebuilt). But this includes JITted modules (e.g., when
running AppleObjectiveCRuntimeV2 utility functions) in which case we wouldn’t
want to flush the type systems. Also the Clang REPL (and I assume the Swift
REPL) rely on the type system being still present after we unloaded the module
associated with the evaluated expression. All this is to say, I found it to be
quite fiddly to determine when to flush the persistent variables from within
that notification. Really we would like a notification to the Target which says
“we restarted the debugee AND some module got rebuilt”. In that case it’s not
safe to keep the persistent variables around. I’ll double check if there isn’t
something like that around.
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
