[Lldb-commits] [PATCH] D139158: [LLDB][LoongArch] Make software single stepping work

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Looks mechanically fine. These classes are all a bit of a copy paste job right now, so if you were going to take inspiration riscv is your best bet.

On the subject of coding style, https://lldb.llvm.org/resources/contributing.html.

  Coding Style: LLDB’s code style differs from LLVM’s coding style. Unfortunately there is no document describing the differences. Please be consistent with the existing code.

Which isn't super helpful but in general look at something central like `lldb/source/Target/Process.cpp` and see what it does. The main difference you'll see is `variable_names_with_underscores_like_this` and `m_` prefix for class members.

For me, the style here looks fine.



================
Comment at: lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h:58
+                                               uint32_t reg_num) override;
+  lldb::addr_t ReadPC(bool *success);
+  bool WritePC(lldb::addr_t pc);
----------------
I think the older targets use this form but for riscv they went with `llvm::Optional<lldb::addr_t> ReadPC();` which I prefer over pointer plus addr_t.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139158/new/

https://reviews.llvm.org/D139158