Check for legacy genvar declarations and generate region style

[msfschaffner]

Summary

The google and lowRISC style guides discourage the use of generate regions and separate genvar declarations, since they are considered legacy constructs.

It would be nice if the style linter could detect those constructs and flag them.

Suggested rule names: [legacy-genvar-declaration] [legacy-generate-region]

Test cases and examples

The more examples you provide, the easier it will be for someone to come along and implement it.

bad:

genvar k;
// may have code her as well
generate
  for (k = 0; k < FooParam; k++) begin : gen_loop
    // code
  end
endgenerate

generate
  if (FooParam) begin : gen_if
  // code
  end
endgenerate

good:

for (genvar k = 0; k < FooParam; k++) begin : gen_loop
  // code
end

if (FooParam) begin : gen_if
  // code
end

[msfschaffner]

b/170256350

[fangism]

There is a closely related rule [v2001-generate-begin]:

https://cs.opensource.google/verible/verible/+/master:verilog/analysis/checkers/v2001_generate_begin_rule.h

That one checks explicitly for generate begin. I should ask around if that one can be generalized to cover all uses of generate.

[fangism]

In both places I checked, including the lowRISC guide section you cited, I see:

Do not wrap a generate construct with an additional begin block.

Do not use generate regions {generate, endgenerate}.

So they are in agreement.

Of these statements, the second seems to cover the first, making it somewhat redundant. WDYT?

If not redundant, should these two statements be implemented as one rule or separate?

[msfschaffner]

Indeed, the second one seems to be a super set of the first with respect to style guidance (i.e., don't use generate regions). Hence I would be ok with just implementing one rule for this, unless we see a need that this has to be made configurable in the future (and in such a case you may still want both rules).