Gary has commented already, but I thought it might be worth adding a bit of background. Firstly, thank you very much for the contribution! It's really wonderful for us to be having an impact like you describe.

About the technical side of things, I think @nbdd0121 is probably right: we'll end up with a simpler style guide if we just tell people not to have a function that refers to anything that isn't an argument. This will also have the property you want, and disallows other things that are probably not a great idea.

Would you be happy to put together a version like that? We'd be very grateful!

Thanks for the feedback! Yes, it seems that openTitan, Ibex, and CVA6 already follow that stricter rule.

I did find one exception in Ibex, but it is for DV, so it probably doesn't matter:
https://github.com/lowRISC/ibex/blob/f60d03b6b064c38e111931ff9b0bae70d6af9af1/examples/simple_system/rtl/ibex_simple_system.sv#L323-L325

How does this look?

I did find one exception in Ibex, but it is for DV, so it probably doesn't matter:
https://github.com/lowRISC/ibex/blob/f60d03b6b064c38e111931ff9b0bae70d6af9af1/examples/simple_system/rtl/ibex_simple_system.sv#L323-L325

Yup that's fine as it's not used by synthesisable code. It exists to give the testbench an easy way to read out performance counters.

Just squashed. Thanks for all the feedback!

Although I wanted to get your opinion on another issue with functions. VCS will currently not run this correctly:

module test;

function automatic [7:0] test_func;
    test_func = 1;
endfunction

wire [7:0] test_wire = test_func();

initial begin
    #1;
    $display("Expected %h, Recieved %h", test_func(), test_wire);
    // Will fail because test_func() is never run
    $finish;
end

endmodule

Maybe this should be a separate issue, but I wonder if there should be another rule against blanket calling of functions from assign/wire. It works fine if always_comb is used.

Following up on the question about functions and continuous assignments: I didn't know that VCS wasn't happy with that! I just looked at IEEE 1800, and there's no mention of a constraint: the right hand side of a "net declaration assignment" seems to be an arbitrary expression in the syntax, at least.

If we want to think carefully about style guide recommendations for this, maybe we should start a separate issue.

Maybe this should be a separate issue, but I wonder if there should be another rule against blanket calling of functions from assign/wire. It works fine if always_comb is used.

We can probably add this in a separate PR, but it sounds like we should restrict assigning a functions output directly to a wire and mandate that this happens in an always_comb block.

I think I'd prefer to outright ban wire [7:0] test = test_func() type statements. This is because their semantics varies depending upon the net type. For wire you do get the equivilent to a continuous assignment but for logic it's just an initial value setting (much like an initial block though not identical! Those initial assignments are made before any initial blocks are run). Switching wire to logic may otherwise appear innocuous so better to avoid the complexity altogether. I note we explicitly allowed this currently:

Initial assignments can be useful in FPGA but are deadly for ASIC synthesis as it's a great way to get simulation/synthesis mismatch where the FPGA version agrees with the simulation but the ASIC synthesis doesn't. If you confine them to an 'initial block at least it makes it far obvious you have one. We could consider saying initial blocks are only allowed in RTL that is strictly for FPGA usage (e.g. top-levels and wrappers etc that we may use on FPGA to instantiate IP)

I personally don't think we should get rid of the wire shorthand. It is orthogonal to the reported VCS issue.

The VCS issue probably is caused by the function being zero argument (and therefore not sensitive to anything). I think we should ban functions with no input instead. It could be a localparam instead.

I think I'd prefer to outright ban wire [7:0] test = test_func() type statements. This is because their semantics varies depending upon the net type. For wire you do get the equivilent to a continuous assignment but for logic it's just an initial value setting (much like an initial block though not identical! Those initial assignments are made before any initial blocks are run). Switching wire to logic may otherwise appear innocuous so better to avoid the complexity altogether. I note we explicitly allowed this currently:

(pedantic mode on) wire is a "net type" and logic is a "data type". Two different things. This is frequently taught incorrectly in universities, and it's a pet peeve of mine.

Unfortunately, because many, many people have received the same poor instruction, that logic should just be used everywhere and can replace the wire vs. reg confusion, we're left with a lot of bad practices and more confusion. The differences between nets and variables are still relevant.

This language maybe gets a little too cute with defaults:

// Equivalent variable declarations with initial values.
logic foo = bar;
var logic foo = bar;

// Equivalent net declarations with continuous assignment.
wire foo = bar;
wire logic foo = bar;

I just created a new issue for this. We can discuss more here: #67