mshr-h / vscode-verilog-hdl-support

HDL support for VS Code
MIT License
290 stars 76 forks source link

[BUG] Verilator logger doesn't ouput warnings/errors unless they're verilator errors #489

Closed alex-the-new-guy closed 2 months ago

alex-the-new-guy commented 2 months ago

Describe the bug When running a linter there may be errors besides linter tool errors, such as linter executable not being reachable. Linter output then looks like:

2024-07-19 11:38:37.846 [info] [LintManager] [VerilatorLinter] [verilator]   command: C:\devel\FPGA\verilator\bin\verilator_bin.exe --lint-only -I"c:/tmp/vscode_verilog_testenvs/verilator/sources" -Wall -y ${workspaceFolder}/sources "c:/tmp/vscode_verilog_testenvs/verilator/sources/top.v"
2024-07-19 11:38:37.846 [info] [LintManager] [VerilatorLinter] [verilator]   cwd    : c:\work_module\tmp\vscode_verilog_testenvs\verilator
2024-07-19 11:38:37.941 [info] [LintManager] [VerilatorLinter] [verilator] 0 errors/warnings returned

Which doesn't communicate in any way that the install directory is misconfigured and the linter actually didn't run at all. Meanwhile the message about "directory being invalid", or really anything else that might suggest something went wrong and/or help diagnose it just gets ignored. Just duping stderr messages into logger.error would be very much good enough.

Environment (please complete the following information):

Steps to reproduce Steps to reproduce the behavior: In my case:

  1. Set "verilog.liting.linter": "verilator" and "verilog.linting.path": "<some wrong path>"
  2. Launch linting via Verilog: Rerun lint tool
  3. Watch the output

In my case, the output is:

2024-07-19 12:15:05.719 [info] [LintManager] Executing runLintTool()
2024-07-19 12:15:07.323 [info] [LintManager] Using verilator linter
2024-07-19 12:15:07.324 [info] [LintManager] [VerilatorLinter] [verilator] Execute
2024-07-19 12:15:07.324 [info] [LintManager] [VerilatorLinter] [verilator]   command: C:\devel\FPGA\verilator\bin\verilator_bin.exe\verilator_bin.exe --lint-only -I"c:/tmp/vscode_verilog_testenvs/vivado/sources"  "c:/tmp/vscode_verilog_testenvs/vivado/sources/top.v"
2024-07-19 12:15:07.324 [info] [LintManager] [VerilatorLinter] [verilator]   cwd    : c:\tmp\vscode_verilog_testenvs\vivado
2024-07-19 12:15:07.356 [info] [LintManager] [VerilatorLinter] [verilator] 0 errors/warnings returned

Dumping stderr into logger.error would add

2024-07-19 12:28:10.576 [error] [LintManager] [VerilatorLinter] The directory name is invalid.
2024-07-19 12:28:10.576 [error] [LintManager] [VerilatorLinter] 

This, at least, indicates there is an issue. (also, separate regex for errors and warnings would be nice).

mshr-h commented 2 months ago

@alex-the-new-guy Added some error log on v1.14.2. Should be better.

alex-the-new-guy commented 2 months ago

Still doesn't work, output same as the issue.

First of all, if someone puts full bin path, as <verilator_root>/bin/verilator_bin.exe into verilog.linting.path my setup outputs The directory name is invalid into stderr (probably powershell-related or something). That doesn't match any of the predefined messages you've provided. Also, printing "failed to run command" is less helpful then stderr contents since it doesn't tell why the command filed.

IMO a better approach would be to check if stderr line matches some sort of regex (first symbol is % seems good enough in my testing) and outputting whatever there is as error.

Then there are parsing issues. Verilator apparently changes how file path is written, so that instead of <directory>/top.v it gives <directory>\\top.v which is rejected by https://github.com/mshr-h/vscode-verilog-hdl-support/blob/416807d4e70458dfe8b8bf24ca5317f94048c71a/src/linter/VerilatorLinter.ts#L117-L119

Also, something like %Error: $VERILATOR_ROOT needs to be in environment is a legitimate Verilator error, yet it doesn't contain file path at all.

Second issue with this code is that it rejects anything outside of current file. If you have a submodule and proper includes, verilator can parse said includes and check if inputs/outputs are correct.

I suggest just removing the line.indexof(docUri) part

Then there is the regex parser.

I've submitted a PR that attempts to fix these issues. Seems to work on windows and linux (WSL). There may be some edge cases about which I don't know about, but whatever.