[Lldb-commits] [PATCH] D153834: [lldb] Add two-level caching in the source manager

[Alex Langford via Phabricator via lldb-commits]

bulbazord requested changes to this revision.
bulbazord added a comment.
This revision now requires changes to proceed.

LGTM with a few nits. There is one piece of logic that I think does need to be changed before this can go in however.



================
Comment at: lldb/source/Commands/CommandObjectSource.cpp:1223-1224
+    // Dump the process source cache.
+    ProcessSP process_sp = m_exe_ctx.GetProcessSP();
+    if (process_sp) {
+      result.GetOutputStream() << "\nProcess Source File Cache\n";
----------------
nit:


================
Comment at: lldb/source/Commands/CommandObjectSource.cpp:1250-1251
+    // Clear the process cache.
+    ProcessSP process_sp = m_exe_ctx.GetProcessSP();
+    if (process_sp)
+      process_sp->GetSourceFileCache().Clear();
----------------
Same here


================
Comment at: lldb/source/Core/SourceManager.cpp:86
 
-  // It the target source path map has been updated, get this file again so we
-  // can successfully remap the source file
-  if (target_sp && file_sp &&
-      file_sp->GetSourceMapModificationID() !=
-          target_sp->GetSourcePathMap().GetModificationID())
-    file_sp.reset();
+  if (!debugger_sp && debugger_sp->GetUseSourceCache()) {
+    LLDB_LOG(log, "Source file caching disabled: creating new source file: {0}",
----------------
This should be `if (debugger_sp` right? If `debugger_sp` is `false` (because it's nullptr) then you'll end up running `debugger_sp->GetUseSourceCache()` and die.


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

https://reviews.llvm.org/D153834