-
Notifications
You must be signed in to change notification settings - Fork 195
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
[proper-parameter-declaration] parameter vs localparam in packages #2160
Comments
I think the behaviour is intentional. https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#constants: |
Thanks for the link, I guess that means there is a case for configuration. I still think the default for parameters in packages should be changed to localparam based on the LRM. Specifically as compilers will treat parameters in packages as localparam. Better to be explicit. |
Oops wrong button.. |
Yeah, the configuration flag is the safest bet. I don't have an opinion on the default itself, but changing it might be painful for some people |
Adding @msfschaffner who might have insight in the lowRISC style guide. From your experience in SystemVerilog projects, is it typical to have |
Describe the bug
After fixing a few violations from [proper-parameter-declaration] wrt checking parameters in packages, my synthesis tool (Quartus PRO) throws a warning. It seems Quartus expects "localparam" instead of "parameter". Usually I think Quartus is wrong, but after reviewing the LRM I think Quartus is right.
Section 6.20.4 in the LRM (2017 and older) states "parameters" in packages are treated as a synonym for a localparam, so tools should treat parameters as localparam's in a package. More intuitively, as parameters are overidable, a parameter in a package dosn't make sense in that packages are constant and not parameterisable.
I propose the rule default be changed to raise a violation when a parameter is used in a package.
Test case (preferably reduced)
Actual vs. expected behavior
output from verible-verilog-lint
What should have happened?
Expect "parameter" in package to be reported as lint failure.
Citations to published style guides:
System Verilog LRM IEEE Std 1800™-2017, Section 6.20.4
The text was updated successfully, but these errors were encountered: