chipsalliance / verible

Verible is a suite of SystemVerilog developer tools, including a parser, style-linter, formatter and language server
https://chipsalliance.github.io/verible/
Other
1.38k stars 214 forks source link

Formatter: Indent body of initializer lists #352

Closed imphil closed 3 years ago

imphil commented 4 years ago

STR: Run vendor/lowrisc_ip/prim/rtl/prim_keccak.sv (https://github.com/lowRISC/ibex/blob/master/vendor/lowrisc_ip/prim/rtl/prim_keccak.sv) through verible formatter.

Expected output, according to the lowRISC/ibex/opentitan style guide:

localparam logic [63:0] RC[24] = '{
    64'h 0000_0000_0000_0001, // Round 0
    64'h 0000_0000_0000_8082, // Round 1
    64'h 8000_0000_8000_8008 // Round 23
};

Actual output:

localparam logic [63:0] RC[24] = '{64'h0000_0000_0000_0001,  // Round 0
64'h0000_0000_0000_8082,  // Round 1
64'h8000_0000_8000_8008  // Round 23
};

Actually, the https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#line-wrapping isn't very definite on this topic beyond the need for an indent. In line with what Verible typically produces, a four-space indent seems the most appropriate choice.

On the location of the closing };: The lowRISC style guide shows it in the last line in the example, but doesn't explicitly mention that. I opened https://github.com/lowRISC/style-guides/issues/39 to see if there's clear guidance from the style guide. (But let's keep this bug on the indentation for now.)

fangism commented 4 years ago

There are two related issues afoot here, one is the partitioning of the RHS expression, and the other is the indentation thereof.

hzeller commented 3 years ago

Re-opened for now as the fix introduced a regression so disabled for now.