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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to