[Lldb-commits] [PATCH] D76804: Add new LLDB setting: use-source-cache

Thu Mar 26 01:03:18 PDT 2020

labath added a reviewer: jingham.
labath added a comment.

Yeah, the differences between windows&posix handling of memory mapped files is quite annoying (though I can't really say that the windows behavior is worse than randomly sending out SIGBUSes).

The alternative (which is used in other tools like clangd, I believe) is to not use mmap when opening these kinds of files. But I am not sure if that is actually better than not caching at all.

In D76804#1942723 <https://reviews.llvm.org/D76804#1942723>, @emrekultursay wrote:

> I did not see any existing tests that validate setting/reading of properties. Can you point to me if you know any that exists?

`TestSettings.py` does various setting-related things, but I'm not sure how much would reading/writing of this specific setting be valueable, as it would literally only test the single declaration in `CoreProperties.td`. It would be better to have a test which sets this setting to false and then demonstrates that one is able to read/write to the source file. This is something that already works elsewhere, but it would at least test this functionality on windows (and given that we don't have a failing test, we probably don't have a test for this functionality anyway).

================
Comment at: lldb/source/Core/Debugger.cpp:350
+  bool ret = m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
+  SetPrompt(GetPrompt());
+  return ret;
----------------
this looks like copy-paste gone wrong. SetPrompt is present in the use-color setting because that actually affects the value of the prompt -- this doesn't.

But this does raise the question of whether we should nuke the existing cache when this setting is set to false (I think we should).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76804