[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.


================
Comment at: lldb/source/Target/Target.cpp:208
+    // of the debugee.
+    m_scratch_type_system_map.Clear();
     m_process_sp.reset();
----------------
aprantl wrote:
> Michael137 wrote:
> > Michael137 wrote:
> > > 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.
> > I suppose we could do something like:
> > 
> > ```
> > void Target::ModulesDidUnload(ModuleList &module_list, bool delete_locations) {
> >     ...
> >     if each module in module_list is Type::eTypeExecutable or Type::eTypeObjectFile {
> >         m_scratch_type_system_map.Clear();
> >     }
> > ```
> That looks like a reasonable middle ground?
Updated diff with this alternative


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724