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

`IFDEFs and such cause incorrect line numbers #72

Open
MahmoudKMaarouf opened this issue Nov 19, 2022 · 4 comments
Open

`IFDEFs and such cause incorrect line numbers #72

MahmoudKMaarouf opened this issue Nov 19, 2022 · 4 comments

Comments

@MahmoudKMaarouf
Copy link

Placing this on top of code:

`ifndef AN_IF_DEF
logic [1:0] someVar;
`endif

will cause the line numbers of the original file to be shifted 1 up (equivalent to minus 1) more than what it currently is. More lines within the IF_DEF that are not executed will shift it even further, making the lines almost completely useless with large Verilog files and many IF DEFs. This is very hard to work around since the line numbers are critical for my tool.

Any help is appreciated.

@DaveMcEwan
Copy link

@MahmoudKMaarouf Would you add a testcase to sv-parser-pp/src/preprocess.rs?
There are similar testcases called ifdef_undefined and include_noindent.
The important part is the check on, for example, ret.origin(5).unwrap.l (for the 5th character).
If you're not also fixing the bug (it might be difficult), you can add an attribute like #[ignore = "Exposes issue #72, wrong line number."]

https://github.com/dalance/sv-parser/blob/master/sv-parser-pp/src/preprocess.rs#L1213

@MahmoudKMaarouf
Copy link
Author

@DaveMcEwan
Hi Dave, thank you for the suggestions.
I will think about adding these testcases, given that I have time.
May I ask how I run these test cases?

@DaveMcEwan
Copy link

DaveMcEwan commented Nov 21, 2022

Nice and easy :) cd sv-parser-pp/ && cargo test.

The full flow would be something like this:

  • Clone sv-parser: git clone https://github.com/dalance/sv-parser.git
  • Change in to the preprocessor subdirectory: cd sv-parser/sv-parser-pp/
  • Try running the existing testcases: cargo test
  • Start a new branch: git checkout -b issue72
  • Look at how the other tests work, and add one for your case.
  • Run the tests again to include your one: cargo test
    • If you've done some fixes to the logic and your test already passes, you should see cargo report the test as passing.
    • If you're unsure how to fix the logic and the test fails, then add an attribute to ignore the test.
  • Push branch to your fork.
  • Open a PR. It might stay open for a while but at least everybody will have visibility of the issue.

Is that helpful?

@MahmoudKMaarouf
Copy link
Author

Much appreciated Dave.
In the coming weeks, I will put some effort into this, and hopefully come up with a fix, or at least a testcase.

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

No branches or pull requests

2 participants