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

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added inline comments.


================
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:1002
+  assert(m_instr_info && m_reg_info && m_subtarget_info && m_asm_info &&
+         m_context && disasm && instr_printer);
+}
----------------
labath wrote:
> this should be `m_disasm` and `m_instr_printer` (and make sure to test this with assertions enabled).
Thanks, I was not aware of this CMake option. Of course assertion failed.


================
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:102
+  // disassemblers.
+  class MCDisasmToolset;
+  std::unique_ptr<MCDisasmToolset> m_disasm;
----------------
clayborg wrote:
> Maybe "MCDisasmInstance", "MCDisassembler" or just "Disassembler"?
MCDisassembler already exists in llvm namespace (it is one of members of this class).   Disassembler is base class of DisassemblerLLVMC. l thought about MCDisasmImpl also. Probably MCDisasmInstance is even better.


https://reviews.llvm.org/D41584