Refactor string conversion for InstructionControlFlowKind enum

[jj10306]

Summary

Refactor the string conversion of the lldb::InstructionControlFlowKind enum out
of Instruction::Dump to enable reuse of this logic by the
JSON TraceDumper (to be implemented in separate diff).

Will coordinate the landing of this change with D130320 since there will be a minor merge conflict between
these changes.

Test Plan:
Run unittests

> ninja check-lldb-unittests
[4/5] Running lldb unit test suite

Testing Time: 10.13s
  Passed: 1084

Verify '-k' flag's output

(lldb) thread trace dump instructions -k
thread #1: tid = 1375377
  libstdc++.so.6`std::ostream::flush() + 43
    7048: 0x00007ffff7b54dab    return      retq
    7047: 0x00007ffff7b54daa    other       popq   %rbx
    7046: 0x00007ffff7b54da7    other       movq   %rbx, %rax
    7045: 0x00007ffff7b54da5    cond jump   je     0x11adb0                  ; <+48>
    7044: 0x00007ffff7b54da2    other       cmpl   $-0x1, %eax
  libc.so.6`_IO_fflush + 249
    7043: 0x00007ffff7161729    return      retq
    7042: 0x00007ffff7161728    other       popq   %rbp
    7041: 0x00007ffff7161727    other       popq   %rbx
    7040: 0x00007ffff7161725    other       movl   %edx, %eax
    7039: 0x00007ffff7161721    other       addq   $0x8, %rsp
    7038: 0x00007ffff7161709    cond jump   je     0x87721                   ; <+241>
    7037: 0x00007ffff7161707    other       decl   (%rsi)
    7036: 0x00007ffff71616fe    cond jump   je     0x87707                   ; <+215>
    7035: 0x00007ffff71616f7    other       cmpl   $0x0, 0x33de92(%rip)      ; __libc_multiple_threads
    7034: 0x00007ffff71616ef    other       movq   $0x0, 0x8(%rsi)
    7033: 0x00007ffff71616ed    cond jump   jne    0x87721                   ; <+241>
    7032: 0x00007ffff71616e9    other       subl   $0x1, 0x4(%rsi)
    7031: 0x00007ffff71616e2    other       movq   0x88(%rbx), %rsi
    7030: 0x00007ffff71616e0    cond jump   jne    0x87721                   ; <+241>
    7029: 0x00007ffff71616da    other       testl  $0x8000, (%rbx)           ; imm = 0x8000

[jj10306]

remove accidentily included change to trace dumper

lldb/source/Target/TraceDumper.cpp

221 ↗

(On Diff #447723)

ignore this accidentally included change, will delete this and update the diff

[jj10306]

lldb/source/Core/Disassembler.cpp

602

Thoughts on adding a default case with a string that indicates that no string conversion was implemented for the provided variant?
This would potentially make it easier for someone to realize they didn't add a case here for a newly added variant of InstructionControlFlowKind. Otherwise undefined data would be returned if a new case isn't added for a new variant.
wdyt?

[persona0220]

lldb/source/Core/Disassembler.cpp

602

I agree to add a default here!

[wallace]

just remove that comment. I've just pushed Sujin's diff as well, so you can rebase

lldb/include/lldb/lldb-enumerations.h

975–976 ↗

(On Diff #447727)

you can safely remove this comment because the compiler will complain if a new enum value is added and the switch case in GetNameForInstructionControlFlowKind is not updated

lldb/source/Core/Disassembler.cpp

602

no defaults for enums, otherwise the compiler won't complain if a new enum value is added and a case is not added here

[jj10306]

Rebase, fix merge conflicts with D130320

[jj10306]

Remove unnecessary comment

lldb/include/lldb/Core/Disassembler.h

public:
   };
 
   virtual bool ParseOperands(llvm::SmallVectorImpl<Operand> &operands) {
     return false;
   }
 
   virtual bool IsCall() { return false; }
 
+  static const char *GetNameForInstructionControlFlowKind(
+      lldb::InstructionControlFlowKind instruction_control_flow_kind);
+
 protected:
   Address m_address; // The section offset address of this instruction
                      // We include an address class in the Instruction class to
                      // allow the instruction specify the
                      // AddressClass::eCodeAlternateISA (currently used for
                      // thumb), and also to specify data (AddressClass::eData).
                      // The usual value will be AddressClass::eCode, but often
                      // when disassembling memory, you might run into data.

lldb/source/Core/Disassembler.cpp

 Instruction::~Instruction() = default;
 
 AddressClass Instruction::GetAddressClass() {
   if (m_address_class == AddressClass::eInvalid)
     m_address_class = m_address.GetAddressClass();
   return m_address_class;
 }
 
+const char *Instruction::GetNameForInstructionControlFlowKind(
+    lldb::InstructionControlFlowKind instruction_control_flow_kind) {
+  switch (instruction_control_flow_kind) {
+  case eInstructionControlFlowKindUnknown:
+    return "unknown";
+  case eInstructionControlFlowKindOther:
+    return "other";
+  case eInstructionControlFlowKindCall:
+    return "call";
+  case eInstructionControlFlowKindReturn:
+    return "return";
+  case eInstructionControlFlowKindJump:
+    return "jump";
+  case eInstructionControlFlowKindCondJump:
+    return "cond jump";
+  case eInstructionControlFlowKindFarCall:
+    return "far call";
+  case eInstructionControlFlowKindFarReturn:
+    return "far return";
+  case eInstructionControlFlowKindFarJump:
+    return "far jump";
+  }
+}
+
 void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size,
                        bool show_address, bool show_bytes,
                        bool show_control_flow_kind,
                        const ExecutionContext *exe_ctx,
                        const SymbolContext *sym_ctx,
                        const SymbolContext *prev_sym_ctx,
                        const FormatEntity::Entry *disassembly_addr_format,
                        size_t max_address_text_size) {
if (m_opcode.GetType() == Opcode::eTypeBytes) {
       if (max_opcode_byte_size > 0)
         m_opcode.Dump(&ss, max_opcode_byte_size * 3 + 1);
       else
         m_opcode.Dump(&ss, 12);
     }
   }
 
   if (show_control_flow_kind) {
-    switch (GetControlFlowKind(exe_ctx)) {
-    case eInstructionControlFlowKindUnknown:
-      ss.Printf("%-12s", "unknown");
-      break;
-    case eInstructionControlFlowKindOther:
-      ss.Printf("%-12s", "other");
-      break;
-    case eInstructionControlFlowKindCall:
-      ss.Printf("%-12s", "call");
-      break;
-    case eInstructionControlFlowKindReturn:
-      ss.Printf("%-12s", "return");
-      break;
-    case eInstructionControlFlowKindJump:
-      ss.Printf("%-12s", "jump");
-      break;
-    case eInstructionControlFlowKindCondJump:
-      ss.Printf("%-12s", "cond jump");
-      break;
-    case eInstructionControlFlowKindFarCall:
-      ss.Printf("%-12s", "far call");
-      break;
-    case eInstructionControlFlowKindFarReturn:
-      ss.Printf("%-12s", "far return");
-      break;
-    case eInstructionControlFlowKindFarJump:
-      ss.Printf("%-12s", "far jump");
-      break;
-    }
+    lldb::InstructionControlFlowKind instruction_control_flow_kind =
+        GetControlFlowKind(exe_ctx);
+    ss.Printf("%-12s", GetNameForInstructionControlFlowKind(
+                           instruction_control_flow_kind));
   }
 
   const size_t opcode_pos = ss.GetSizeOfLastLine();
 
   // The default opcode size of 7 characters is plenty for most architectures
   // but some like arm can pull out the occasional vqrshrun.s16.  We won't get
   // consistent column spacing in these cases, unfortunately.
   if (m_opcode_name.length() >= opcode_column_width) {