clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed.
================ Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:77-83 + std::unique_ptr<llvm::MCInstrInfo> m_instr_info; + std::unique_ptr<llvm::MCRegisterInfo> m_reg_info; + std::unique_ptr<llvm::MCSubtargetInfo> m_subtarget_info; + std::unique_ptr<llvm::MCAsmInfo> m_asm_info; + std::unique_ptr<llvm::MCContext> m_context; + std::unique_ptr<llvm::MCDisassembler> m_disasm; + std::unique_ptr<llvm::MCInstPrinter> m_instr_printer; ---------------- add "_up" suffix to all std::unique_ptr member variables. ================ Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:989-1003 +DisassemblerLLVMC::MCDisasmToolset::MCDisasmToolset( + std::unique_ptr<llvm::MCInstrInfo> &&instr_info, + std::unique_ptr<llvm::MCRegisterInfo> &®_info, + std::unique_ptr<llvm::MCSubtargetInfo> &&subtarget_info, + std::unique_ptr<llvm::MCAsmInfo> &&asm_info, + std::unique_ptr<llvm::MCContext> &&context, + std::unique_ptr<llvm::MCDisassembler> &&disasm, ---------------- Do we need this? We are placing the MCDisasmToolset into std::unique_ptr<> member variables so this shouldn't be needed by anyone as the std::unique_ptr already has a move operator ================ Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:102 + // disassemblers. + class MCDisasmToolset; + std::unique_ptr<MCDisasmToolset> m_disasm; ---------------- Maybe "MCDisasmInstance", "MCDisassembler" or just "Disassembler"? ================ Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:103 + class MCDisasmToolset; + std::unique_ptr<MCDisasmToolset> m_disasm; + std::unique_ptr<MCDisasmToolset> m_alternate_disasm; ---------------- add "_up" suffix to anything that is a std::unique_ptr ================ Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:104 + std::unique_ptr<MCDisasmToolset> m_disasm; + std::unique_ptr<MCDisasmToolset> m_alternate_disasm; }; ---------------- ditto https://reviews.llvm.org/D41584 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
