verible-lint GH Action reports violation for reggen-generated code

[imphil]

With the verible-lint GH Action now enabled in reviews we get errors like the following one:

⚠️ [verible-verilog-lint] _{reported by reviewdog :dog:}
struct definitions always should be named using typedef. [Style typedef-structs-unions] [typedef-structs-unions]

This code is generated by reggen. We could either fix reggen to produce lint-passing code, or disable lint (or only the action?) for this piece of code.

See also https://github.com/lowRISC/opentitan/pull/8845

[rswarbrick]

I vote for disabling lint on (this) autogenerated code.

[imphil]

Looking closer, this is an issue with the GH Action not picking up the same configuration as we use for the Verible lint going through dvsim.

We do have an explicit rule for the typedef-structs-unions message: https://github.com/lowRISC/opentitan/blob/b19dec1b90e724341fbd24f2646d96d1e968407f/hw/lint/tools/veriblelint/rules.vbl#L16

dvsim roughly runs this command:

verible-verilog-lint --lint_fatal --parse_fatal --rules_config=../src/lowrisc_lint_common_0.1/tools/veriblelint/rules.vbl --waiver_files=../src/lowrisc_ip_lc_ctrl_pkg_0.1/lint/lc_ctrl_pkg.vbl ../src/lowrisc_ip_aes_0.6/rtl/aes.sv

Here's the report from dvsim: Report here

While the GH Action runs this command (for an arbitrary file):

find . -type f -name '*.vbl' -exec cat {} \; > verible_config
verible-verilog-lint --rules_config verible_config ./hw/ip/flash_ctrl/rtl/flash_ctrl_pkg.sv

Things to note:

• I can't see what verible_config as produced by the action really contains. It would be nice to cat this file in the GH action before it's used.
• In the GH Action we combine waivers and rules into a single file. I'm not sure how Verible deals with that. (If we need to differentiate we should prioritize #8850.)
• It would be helpful for the GH Action to align its naming to Verible: the config file should be a rules config.

@tgorochowik Do you have time to look into that soon-ish? Otherwise we can disable the GH Action for the moment to avoid too many false positives.

[tgorochowik]

I created #8851 to partially address those issues, before a proper fix is implemented.

I vote for disabling lint on (this) autogenerated code.

There is an exclude_paths param in the action that can be used for this assuming the paths to auto-generated code is known

[imphil]

8851 should fix the issue for the moment.

I'll work on #8850 next to clearly differentiate between waivers and rules, which need to be treated separately.

[imphil]

We now have a Verible lint config in the GH Action that matches the one used in Azure Pipelines (where Verible lint is run through fusesoc/dvsim). That's good enough to solve the problem at hand.