search

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.34k stars 205 forks source link

invalid-system-task-function not checked if multiple ifdefs are present in file #2256

Open matlupi opened 2 weeks ago

matlupi commented 2 weeks ago

Describe the bug

The rule [invalid-system-task-function] is not reported if the file contains multiple ifdefs

Test case (preferably reduced)

module test;
  logic a;
  logic clk;

`ifdef A
  c
  `ifndef B
    #(.A(1))
  `endif
  dut(
  .clk(clk)
      );
`endif

  `ifdef C
  initial
    a = $random();
  `endif
endmodule

Actual vs. expected behavior

False-positive? False-negative? Crash? Wrong diagnostic location?

False negative

What should have happened?

test.sv:17:9-15: $random is a forbidden system function or task, please use $urandom instead [Style: forbidden-system-functions] [invalid-system-task-function]

What happens

Error not reported for line 17.

Tested on versions

NOTE: If the ifndef B is removed (lines 7 and 9) the error is correctly flagged.

matlupi commented 1 week ago

@hzeller, could you point me to someone who could point me in the right direction to address this one?

hzeller commented 1 week ago

It does look strange. Is the C macro defined ? Because only then that branch would be seen. I suspect the root problem is, that our preprocessor is implemented not very fully featured...

I have a bunch of TODOs that I want to implement w.r.t. the preprocessor but there is also a limited time at night and weekends :)

Anyway, a good starting point to debug is printing the token streams using verible-verilog-syntax and then go from there. IIRC, the syntax check has the preprocessor enabled, but I'd have to look at the code to confirm.

matlupi commented 3 days ago

It does look strange. Is the C macro defined ? Because only then that branch would be seen.

Thanks for the input @hzeller.

The macros can be defined but in general, when running a linting on the design, it is important to make sure that the checks are independent of the macros defined.

In the specific case, however, the check is not performed w/ the provided code. The check is performed, however, if the first block of ifdef A is not present.

I'll look into what you proposed.