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

Document parse-order-dependency for config files #1474

Closed
veripoolbot opened this issue Jun 27, 2019 · 8 comments
Closed

Document parse-order-dependency for config files #1474

veripoolbot opened this issue Jun 27, 2019 · 8 comments

Comments

@veripoolbot
Copy link
Contributor

@veripoolbot veripoolbot commented Jun 27, 2019


Author Name: Philipp Wagner
Original Redmine Issue: 1474 from https://www.veripool.org

Original Assignee: Philipp Wagner


When including a .vlt configuration file with lint waivers, only the
global waivers (file == "
") are applied unconditionally; all
file/line-specific waivers are only applied to files parsed after the
configuration file has been parsed.

This behavior is not documented so far (and took me some time to figure
out), so add a small note in the documentation.

Please find a patch here:
https://github.com/imphil/verilator/commit/f937318e14a454ce644645c368b415ae3ea280ec.patch

I'm not sure if you prefer to get pull requests on GitHub now, let me know and I'll do one over there.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jun 27, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-06-27T22:00:55Z


Sorry you had to experiment to figure this out (I didn't realize myself ;) but major thanks for improving the docs.

Trivial really for a doc patch, but so you're set for the future, to the patch please insert your name in docs/CONTRIBUTORS to acknowledge this and future contributions are made under the Developer Certificate of Origin (https://developercertificate.org/). (Needed just once.)

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jul 6, 2019


Original Redmine Comment
Author Name: Philipp Wagner
Original Date: 2019-07-06T20:46:34Z


Hi Wilson,

Sorry for the slow response, looks like the email notification didn't reach me.

An updated patch with a DCO Signed-off-by line is available here: https://github.com/imphil/verilator/commit/836acde22b59685a17a945d37346419ee3996523.patch

And a patch adding my name to the contributors list is here: https://github.com/imphil/verilator/commit/4558efc7151bebabfd37f69b62d0764b3450ee6a.patch

(Both patches are also in my note-config-parse-order branch at https://github.com/imphil/verilator.git

Thanks!

Philipp

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jul 6, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-07-06T22:11:04Z


Perfect, thanks, pushed to git towards 4.018.

@veripoolbot veripoolbot closed this Jul 6, 2019
@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jul 8, 2019


Original Redmine Comment
Author Name: Shareef Jalloq
Original Date: 2019-07-08T13:34:01Z


I've hit what I think is the same issue today but can't get the waivers to apply. I'm supplying the .wlt configuration file before any Verilog files or paths. I'm using v4.016 and compiled from source. My commandline follows. Is this not the way to fix the issue?

Thanks.

lint:
	verilator --lint-only -Wall \
	$(MODULE).vlt \
	-y $(PROJ_HOME)/src/misc/verilog \
	-y $(PROJ_HOME)/src/regfile/verilog \
	-y $(PROJ_HOME)/src/spis/verilog \
	-y $(PROJ_HOME)/src/top/verilog \
	$(MODULE).v > $(MODULE).log 2>&1
</code>
@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jul 8, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-07-08T13:43:10Z


Yes, the .vlt should be first. If you don't see suppression working please reduce it to a test case and file a new bug, thanks.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jul 8, 2019


Original Redmine Comment
Author Name: Shareef Jalloq
Original Date: 2019-07-08T15:00:47Z


Creating the testcase shows me that you require an absolute path in the VLT file. Is that by design or a bug?

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jul 9, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-07-09T02:15:46Z


An absolute path wasn't really intended but isn't surprising since it was never thought about - would you be willing to make a new issue and try a patch, you can see preprocOpen in V3PreShell.cpp for an example of how to call filePath to resolve this.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jul 9, 2019


Original Redmine Comment
Author Name: Shareef Jalloq
Original Date: 2019-07-09T11:21:04Z


Thanks, will give it a go when I have time. Cheers.

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

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.