[Lldb-commits] [PATCH] D136795: [LLDB] Add a `(not loaded) ` prefix to placeholder object filename to indicate they are not loaded.

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 3 07:37:58 PDT 2022


labath added a comment.

That's exactly why i don't like this stringly typed `GetPluginName` API. If you had to write `isa<ProcessMinidump::Placeholder>(objfile)`, or even just `objfile->GetPluginName()==ProcessMinidump::Placeholder::GetPluginNameStatic()` , you'll be a lot less likely to think that you can get away with it.

You weren't around then, but back when this "plugin" was introduced, I made the point that the functionality is not really minidump-specific -- and it was dismissed because it was "experimental".  At some point, I think it stopped being that.

Since this is essentially adding support for the placeholder concept into the generic code, why not just admit that, and move the code somewhere else. That'll open up a lot of other cool features as well.

I expect that when the user starts seeing this "placeholder" annotation, his next request will be like "oh, but I know where to find that file -- why don't you let me load it?". Well, if we had first class support for this, we could add a command that does just that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136795



More information about the lldb-commits mailing list