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

Support for Comment-Based Warning Control #977

Open
hankhsu1996 opened this issue Apr 28, 2024 · 2 comments
Open

Support for Comment-Based Warning Control #977

hankhsu1996 opened this issue Apr 28, 2024 · 2 comments

Comments

@hankhsu1996
Copy link

Is your feature request related to a problem? Please describe.

Currently, slang supports suppressing warnings using pragma directives like:

`pragma diagnostic push
`pragma diagnostic ignore="-Wempty-member"
// some code that will issue warning without the pragma
`pragma diagnostic pop

However, the `pragma directive is not supported in VCS and result in errors such as:

Error-[UM] Undefined macro
test.sv, 1
  Undefined macro exists as: 'pragma'

This discrepancy leads to complications in managing code compatibility across different tools.

Describe the solution you'd like

I propose adding an alternative method to control warnings with comments, similar to how clang family handle it. For example, clang-format has // clang-format off, clang-tidy has // NOLINTNEXTLINE.

This approach would allow for more compatibility with tools that do not support pragma directives.

@MikePopoloski
Copy link
Owner

I'm always skeptical of tools doing any kind of parsing of comments; it feels like a violation of what comments are supposed to be. At least in this case it seems less bad since it's just controlling diagnostic output at the end of the pipeline; comments that modify actual semantics are way worse.

Note that it's possible already today to avoid the problem you're describing via simple ifdefs:

`ifdef __slang__
`pragma diagnostic ignore="-Wempty-member"
`endif

That should prevent other tools from complaining. I realize it's a bit verbose, though you can hide it behind a macro:

`ifdef __slang__
`define SLANG_IGNORE(x) `pragma diagnostic ignore=x
`else
`define SLANG_IGNORE(x)
`endif

@hankhsu1996
Copy link
Author

I prefer not to use comments for controlling tool behavior either. However, if we're considering simpler one-line diagnostic controls in the future (something like logic my_var; // -Wno-unassigned-variable), comments might be our only option as a pragma can't follow other code on the same line (correct me if I'm wrong). This approach essentially acts like a shorthand for push, disable, and pop. Many popular linters, such as pylint and eslint, implement the diagnostic control this way.

It's odd that some commercial tools still don't support pragma, especially since it has been in the Verilog spec since before 2005. Here’s a quick check on pragma support in various tools:

  • Aldec Riviera Pro 2023.04: Supported
  • Cadence Xcelium 23.09: Not supported
  • Siemens Questa 2023.3: Supported
  • Synopsys VCS 2023.03: Not supported
  • Icarus Verilog 12.0: Supported
  • Yosys 0.37: Not supported

Whether to support comment-based control might depend on the need for compatibility with other tools. If maintaining the current approach to do things correctly is important, I understand and respect that. But if compatibility is a higher priority, then using comments might be the way to go.

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

No branches or pull requests

2 participants