clang-format proposal and patch to allow re-org includes

[renau]

I committed a src/.clang-format with a uniform style suggestion for dromajo.

I did not run the clang-format, but to run it. cd src clang-format -i .h .cpp git diff .

The small include patch is to allow reorg the includes by name (clang-format does it)

I think that it is a clean format style, and it will make the code more uniform.

[et-tommythorn]

There's pros and cons of course. The major cons is that it makes it harder to pull in things that were removed, but on the balance is probably not enough of a concern.

[tommythorn]

Now that I have actually [partially] reviewed the output it, I'm very impressed overall.

I do have some major objections:

• Two space indentation makes it harder to visually align blocks. Linu[sx] goes all the way to 8, but I generally prefer the compromise 4.
• The line length of 132 is excessive. I wouldn't want more than, say 106 (halfway between 80 and 132).

Less critical, I also would like to point out where this fails:

• the formatting in virt_machine_main is very deliberate and IMO is destroyed by clang-format. Contrast:

      static struct option long_options[] = {
          {"cmdline",                 required_argument, 0,  'c' }, // CFG
          {"ncpus",                   required_argument, 0,  'n' }, // CFG
          {"load",                    required_argument, 0,  'l' },
          {"save",                    required_argument, 0,  's' },
          {"maxinsns",                required_argument, 0,  'm' }, // CFG
          {"trace   ",                required_argument, 0,  't' },
          {"ignore_sbi_shutdown",     required_argument, 0,  'P' }, // CFG
          {"dump_memories",           required_argument, 0,  'D' }, // CFG
          {"memory_size",             required_argument, 0,  'M' }, // CFG
          {"memory_addr",             required_argument, 0,  'A' }, // CFG
          {"bootrom",                 required_argument, 0,  'b' }, // CFG
          {"compact_bootrom",               no_argument, 0,  'o' },
          {"reset_vector",            required_argument, 0,  'r' }, // CFG
          {"dtb",                     required_argument, 0,  'd' }, // CFG
          {"mmio_range",              required_argument, 0,  'R' }, // CFG
          {"plic",                    required_argument, 0,  'p' }, // CFG
          {"clint",                   required_argument, 0,  'C' }, // CFG
          {"custom_extension",              no_argument, 0,  'u' }, // CFG
          {0,                         0,                 0,  0 }
      };

  with

      static struct option long_options[] = {{"cmdline", required_argument, 0, 'c'},  // CFG
                                             {"ncpus", required_argument, 0, 'n'},    // CFG
                                             {"load", required_argument, 0, 'l'},
                                             {"save", required_argument, 0, 's'},
                                             {"maxinsns", required_argument, 0, 'm'},  // CFG
                                             {"trace   ", required_argument, 0, 't'},
                                             {"ignore_sbi_shutdown", required_argument, 0, 'P'},  // CFG
                                             {"dump_memories", required_argument, 0, 'D'},        // CFG
                                             {"memory_size", required_argument, 0, 'M'},          // CFG
                                             {"memory_addr", required_argument, 0, 'A'},          // CFG
                                             {"bootrom", required_argument, 0, 'b'},              // CFG
                                             {"compact_bootrom", no_argument, 0, 'o'},
                                             {"reset_vector", required_argument, 0, 'r'},  // CFG
                                             {"dtb", required_argument, 0, 'd'},           // CFG
                                             {"mmio_range", required_argument, 0, 'R'},    // CFG
                                             {"plic", required_argument, 0, 'p'},          // CFG
                                             {"clint", required_argument, 0, 'C'},         // CFG
                                             {"custom_extension", no_argument, 0, 'u'},    // CFG
                                             {0, 0, 0, 0}};

  I imagine there's a way to tell clang-format to leave this one alone.

• Fabrice plays a funny trick with the predecessor, eg: dromajo_fp_template.h and unsurprising clang-format gets confused, but it's not too bad.

• The function arguments are IMO less readable in one big line (the Rust default is the opposite here). Eg. I don't agree that changing

  static inline void handle_dut_overrides(RISCVCPUState *s,
                                      uint64_t mmio_start,
                                      uint64_t mmio_end,
                                      int priv,
                                      uint64_t pc, uint32_t insn,
                                      uint64_t emu_wdata,
                                      uint64_t dut_wdata)
  {
  int opcode = insn & 0x7f;

  to

  static inline void handle_dut_overrides(RISCVCPUState *s, uint64_t mmio_start, uint64_t mmio_end, int priv, uint64_t pc,
                                      uint32_t insn, uint64_t emu_wdata, uint64_t dut_wdata) {
  int opcode = insn & 0x7f;

  is an improvement (I would have expected it to move uint32_t insn, to its own line). However, I can live with this.

I haven't finished reviewing everything yet.