Linter allows commas in instance parameters where they are not allowed in Verilog

[paul-demo]

Verilator's linter is fine with the following trailing comma in parameter list. Most other tools, and I think the language Verilog in general, do not allow a comma after PARAM_A here:

  some_module #(
      .PARAM_A(1),
  ) some_module_inst (
      .clk(clk),
      .reset(reset)
  );

I can probably fix it if you point me to the right files to change.

[wsnyder]

If you could create a pull to add a test (e.g. t_param_comma_bad.v/.pl/out) that would be appreciated. Please also check for a similar problem with class parameters.

Empty pins are allowed in the parser and reported later in V3LinkCells, so probably there's a check needed there for parameters.

[paul-demo]

I didn't see a simple way to fix it. Currently, you look for things with name="" and give them some default param name. However, maybe you also need to check if the value even exists or if it's also empty, and throw a PINNOCONNECT error like you do some of the time.

I didn't get to actually fixing it but wrote down the test cases which succeed even though they should fail:
#5012

It happens when you have > 1 parameter, and you specify 1 parameter with a comma suggesting you were also filling out the second parameter (though it isn't included). Hopefully that makes sense; see the .v files in the PR.

[paul-demo]

Note: it may be PINNOTFOUND in V3LinkDot.cpp that needs to be changed. That's at least the error message displayed in the other cases. Specifically, it might be this function that needs to be updated:

verilator/src/V3LinkDot.cpp

Line 2368 in 1c79df8

void visit(AstPin* nodep) override {

Edit: actually now that I think about it some more, probably foundp is returning true for the empty/missing second parameter (the one created from the nothingness after the trailing comma) since there is a second parameter in the module/class definition. Maybe you're right and it shouldn't even be creating the second, empty/default-named parameter in V3LinkCells.

[wsnyder]

Thanks for the test cases & looking into this. I realized we could alternatively change the grammar to fix this, so committed that.