chipsalliance / verible

Verible is a suite of SystemVerilog developer tools, including a parser, style-linter, formatter and language server
https://chipsalliance.github.io/verible/
Other
1.31k stars 199 forks source link

Syntax checker rejects valid Verilog code. #1572

Open jbylicki opened 1 year ago

jbylicki commented 1 year ago

Describe the bug The syntax checker rejects a case where there are variables named int. While it technically is invalid SystemVerilog syntax, it is valid Verilog (since there was no type int, it was introduced in SV), and legacy projects still do use it. Even the smoke-tests have a case (in nontrivial-mips) where there is a syntax error produced because of it.

Additionally, there are more errors produced due to this issue as the parser later rejects other tokens as being invalid.

To Reproduce

module foo #(
  parameter BAR = 21
)(
  input  wire [1:0] int
);
endmodule

Actual behavior:

$ verible-verilog-syntax in.v
in.v:4:21-23: syntax error at token "int"
in.v:6:1-9: syntax error at token "endmodule"

Expected behavior

Definitely it should be mentioned that this is valid only in Verilog, but the input should be checked. While legacy Verilog is not listed as supported, crashing (rc=1) on legacy code bases is not ideal. Do we want to support legacy Verilog? If so, we should probably introduce tooling around it. @hzeller what do you think?

hzeller commented 1 year ago

Dialect support is something we could implement by for instance not parsing int as a TK_int token, but as an identifier and then have the parser figure it out.

Another approach (which might be better) could be to prime the lexer to not parse keywords depending on the dialect (there might be even a bug open for this).

I like the approach the Verilator lexer is doing, selectively lexing things depending on the chosen dialect. Maybe something like that could be useful.

Paging @fangism who probably has thought through some of the options.

fangism commented 1 year ago

Here https://cs.opensource.google/verible/verible/+/master:verilog/parser/verilog.y;drc=18abf1511dd48625c25028f67910a71d4e27fcba;l=759 you can find a collection of keywords that can also be identifiers in some contexts. In some easy cases, you could allow a keyword like int to be an identifier. I suspect that int may be tricky because types appear in many places in the grammar.