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

Adds column number to the verilator verilog linter #3098

Merged
merged 1 commit into from Apr 18, 2020

Conversation

tarikgraba
Copy link
Contributor

Since version 4.032 (04/2020) verilator linter messages also contain the
column number, and look like:

%Error: /tmp/test.sv:3:1: syntax error, unexpected endmodule, expecting ';'

To stay compatible with old versions of the tool, the column number is
optional in the researched pattern regular expression.

See commit:
verilator/verilator@81c6599

@tarikgraba
Copy link
Contributor Author

Any clue why the tests have failed?

@tarikgraba
Copy link
Contributor Author

@w0rp Do I need to add something?

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test with a message that includes the column number in a message, so we can check that we extract that from the text.

Since version 4.032 (04/2020) verilator linter messages also contain the
column number, and look like:

%Error: /tmp/test.sv:3:1: syntax error, unexpected endmodule, expecting ';'

To stay compatible with old versions of the tool, the column number is
optional in the researched pattern regular expression.

See commit:
verilator/verilator@81c6599
@tarikgraba
Copy link
Contributor Author

tarikgraba commented Apr 18, 2020

@w0rp Vader test added

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

@w0rp w0rp merged commit 64b9a27 into dense-analysis:master Apr 18, 2020
@w0rp
Copy link
Member

w0rp commented Apr 18, 2020

Cheers! 🍻

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

Successfully merging this pull request may close these issues.

None yet

2 participants