Fmt: optimize format rule of let_binder, if_stmt, [x..y]

[trdthg]

• Optimizing format rule of let_binder, if_stmt, [x.... .y] ... and their combinations
• if_stmt formatting rule change: if any of [i, t, e] wrap, then both [t, e] force to wrap
• add more nowrap rule
  • atom
  • string_literal
  • field
  • app
  • binary
  • index
  • if_stmt if all of [i, t, e] can be nowrap
  • trailing comment
  • delim

I also tested it on sail-riscv/model/*.sail

You can see the formatting results compared to sail2 branch at trdthg/sail-riscv#2

There's still a lot of work to be done, but that will be next time

and share a blog I read a few days ago:: https://laurent.le-brun.eu/blog/the-story-of-reformatting-100k-files-at-google-in-2011

[github-actions]

Test Results

    9 files  ±0     21 suites  ±0   0s ⏱️ ±0s
  667 tests +2    667 ✅ +2  0 💤 ±0  0 ❌ ±0 
2 116 runs  +2  2 115 ✅ +2  1 💤 ±0  0 ❌ ±0 

Results for commit 85d3347. ± Comparison against base commit 5d35b1a.

This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.

wrap_into_oneline.sail

if_stmt_1.sail
if_stmt_2.sail
wrap.sail

♻️ This comment has been updated with latest results.

[arichardson]

I had another look at the diff and a lot of it is definitely an improvement, but there is a bit of weirdness with more complex expressions inside the if condition.

[trdthg]

Update

1. Allow Infix_sequence nowrap, this will fix things like

   if
       i > end_element | i >= real_num_elem
     then {

a + b + c + d
[][][][][]
f(f(f(f(f(f(f(f(), f()), f()))))))
if f(a, f(a, b)) then { f(a, f(f(a, b), f(a, b))) } else { .................................... }

all these can be collapsed into one line now

I put some restrictions on purpose before.

For example, a maximum of two levels of nesting f(f()) to avoid situations that would make a line too long like f(f(f(f(f(f())))))))). But it's hard to quantify.

And I found that you can configure max_width in sail_config.json, and ocaml PPrint will automatically handle line breaks. So, I've removed these restrictions now.

[trdthg]

Update:

1. force exps in toplevel function block wrap

   -function clause execute SINVAL_VMA(rs1, rs2) = { execute(SFENCE_VMA(rs1, rs2)) }
   +function clause execute SINVAL_VMA(rs1, rs2) = {
   +  execute(SFENCE_VMA(rs1, rs2))
   +}

[trdthg]

Update:

• Allow Struct, Tuple, Unary nowrap

• Allow binary_rhs(when block) no prefix hardline

  i.e.

   let a = match {
   	...
   }
  

[arichardson]

Can't comment on the OCaml code, but the resulting sail formatting is IMO a big improvement.

Mostly unrelated note: Some of the one-line if/else clauses are a bit complicated and I'd personally add line breaks or maybe set the column limit lower. Is there a way to force multiline if/else? Maybe there should be a config option to say don't merge if/else into a single line unless it's part of a let/other expression?

So basically this is good in one line: let trace_size = if rvfi_int_data_present then { trace_size + 320 } else { trace_size }
But maybe

function hex_bits_signed_forwards bv = {
  if signed(bv) < 0 then { (length(bv), concat_str("-", hex_str(unsigned(not_vec(bv) + 1)))) } else {
    (length(bv), hex_str(unsigned(bv)))
  }
}

should be

function hex_bits_signed_forwards bv = {
  if signed(bv) < 0 then {
    (length(bv), concat_str("-", hex_str(unsigned(not_vec(bv) + 1))))
  } else {
    (length(bv), hex_str(unsigned(bv)))
  }
}

[trdthg]

Thank you for your review. I agree, in fact the comment above mentions a restriction, and it was used to solve this issue (but didn't work well at last)

don't merge if/else into a single line unless it's part of a let/other expression

This seems to be good and should solve the problem here, I'll give it a try

function hex_bits_signed_forwards bv = {
  if signed(bv) < 0 then { (length(bv), concat_str("-", hex_str(unsigned(not_vec(bv) + 1)))) } else {
    (length(bv), hex_str(unsigned(bv)))
  }
}

The reason it's formatted this way now is that this line is wider than 120, so PPrint insert linebreak after else {

If there's a way to make it do the same after then {, that would be nice too

Anyway, I'll try both.

[trdthg]

May need to refactor, so turning it into a Draft for now