> On Jan 9, 2018, at 6:24 AM, Pavel Labath via Phabricator > <revi...@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. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
