[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

[Jason Molenda via lldb-commits]

> On Jan 9, 2018, at 6:24 AM, Pavel Labath via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> labath added a comment.
> 
> In https://reviews.llvm.org/D41584#970945, @tatyana-krasnukha wrote:
> 
>> Thank you, Pavel.
>> Would you mind if I move LLVMCDisassembler declaration in .cpp also? It looks like perfect candidate for pimpl.
> 
> 
> Yes, that sounds like a good idea.
> 
>> And... doesn't DisassemblerLLVMC::LLVMCDisassembler confuse anyone but me?)
> 
> Well... I haven't looked at this class in the past, but yes... it looks confusing. I wouldn't mind a name change.
> 
> Jason, any thoughts on this?


I think the issue is in DisassemblerLLVMC.cpp/.h we have a DisassemblerLLVMC class which implements the Disassembler protocol in lldb and providing the usual Plugin methods (Initialize, Terminate, CreateInstance).

The DisassemblerLLVMC class defines a class itself, LLVMCDisassembler, which is the disassembler (so we can have multiple of them for arm/thumb situations).  I don't think we can hoist the LLVMCDisassembler methods up into DisassemblerLLVMC because we need more than one.  And I'm not great at c++ language lawyer stuff but I don't think we can have a DisassemblerLLVMC class that defines a DisassemblerLLVMC class inside itself.


I'm cool with looking at different names than LLVMCDisassembler, it does look like a mistake more than something done on purpose, but I don't think it's as easy as search & replace unless I've misunderstood.