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

[Greg Clayton via Phabricator via lldb-commits]

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> &&reg_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