Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
matlupi opened this issue Sep 24, 2024 · 3 comments
Labels
style-linter Verilog style-linter issues

Comments

@matlupi
Copy link

matlupi commented Sep 24, 2024

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

  • v0.0-3798-ga602f072
  • v0.0-3428-gcfcbb82b

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

@matlupi matlupi added the style-linter Verilog style-linter issues label Sep 24, 2024
@matlupi
Copy link
Author

matlupi commented Oct 3, 2024

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

@hzeller
Copy link
Collaborator

hzeller commented Oct 3, 2024

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
Copy link
Author

matlupi commented Oct 7, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
style-linter Verilog style-linter issues
Projects
None yet
Development

No branches or pull requests

2 participants