MikePopoloski / slang

SystemVerilog compiler and language services
MIT License
546 stars 114 forks source link

Support for Comment-Based Warning Control #977

Open hankhsu1996 opened 2 weeks ago

hankhsu1996 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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:

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.