Format module port expressions in tabular style

[imphil]

Port expressions in module instantiations should be formatted in tabular style, as discussed in #48.

[sriyerg]

We should also tabular-align parameter set.

[imphil]

Yes, but let's handle that in a separate PR. I'll push that as soon as we have merged this one to avoid the merge conflict.

[sriyerg]

Verible will format this to have each port connection on a new line (which I actually like, since it makes to port connections easier to read).

[mdhayter]

Would it be worth an additional example with a comment rather than a blank line?

mod u_mod (
  .clk_i,
  .rst_ni,
  .sig_i          (my_signal_in),
  .sig2_i         (my_signal_out),
  // comment with no blank line maintains the block
  .in_same_block_i(my_signal_in),
  .sig3_i         (something)
);

[mdhayter]

Maybe "port expressions" rather than just "expressions"? I don't have a strong view.

[imphil]

The SystemVerilog spec calls it "expression", but I also find it too generic. I updated the text to read "port expressions", as suggested.

[imphil]

Thanks for your feedback @sriyerg and @mdhayter, I updated the PR to include your suggestions.

[GregAC]

Do we want to explicitly require tabular alignment, and if so specify a particular style for doing so? As is this just encourages aligning ports but doesn't mandate it and leave it up to the code author to choose a style for doing it.

A strict 'you must do it like this' is problematic because we'll have lots of RTL violating the style guide, but maybe a 'ports must be aligned, style can vary but must be kept consistent across a block and our preferred style(s) is/are ...'?

[imphil]

Do we want to explicitly require tabular alignment, and if so specify a particular style for doing so?

This change is intended to require tabular alignment, which is further described in the "Tabular Alignment" section at the top of the document.

A strict 'you must do it like this' is problematic because we'll have lots of RTL violating the style guide, but maybe a 'ports must be aligned, style can vary but must be kept consistent across a block and our preferred style(s) is/are ...'?

The intend of this clarification is to mandate one way of formatting port expressions, which can then be enforced by tooling.

I do not plan to go around and change all violating RTL code right now, we can do that as we stumble across it, or just rely on Verible format or another tool to do it for us once it is ready. (Which is the same approach we used when introducing clang-format.)

[GregAC]

This change is intended to require tabular alignment, which is further described in the "Tabular Alignment" section at the top of the document.

In that case I think we need stronger wording, the "Tabular Alignment" section starts

Adding whitespace to cause related things to align is encouraged.

Should we switch this to

Tabular alignment of ports of module instantiations is required.
Adding whitespace to cause related things to align elsewhere is encouraged.

With some explanation of the desired style

There must be no trailing or leading whitespace inside the parenthesis containing a port expression. Within a aligned port block there must be no whitespace between the longest port name and the parenthesis.

Then a couple of bad examples to demonstrate violations:

mod u_mod(
  .clk_i,
  .rst_ni,

  // Not allowed, leading/trailing whitespace in parenthesis
  .sig_1_i( sig_1 ),
  .sig_2_i( sig_2 )
);

mod u_mod(
  .clk_i,
  .rst_ni,

  .short_sig_i                       (sig_1),
  // Not allowed, there should be no whitespace between the longest signal name and the parenthesis
  .a_very_long_signal_name_indeed_i  (sig_2)
);

[imphil]

@GregAC good points. I clarified these points, PTAL.

[GregAC]

Looks good, thanks

[imphil]

The discussion in #48 has concluded and we do have a large majority for this change. Let's get it in!