veripool/verilog-mode

verilog-warn-fatal ignored in batch mode

Opened this issue · 1 comments

I noticed that when running in batch mode even when verilog-warn-fatal is set to non-nil, warnings (like unused template lines) are still printed as warnings and not treated as fatal.

Steps to reproduce:

  1. Use a file with an AUTO_TEMPLATE that has an unused line. For example:
.unused        (this_is_unused),
  1. In batch mode, run verilog-batch-auto and confirm that this is printed as a warning.
  2. Set verilog-warn-fatal to t and run again.

Expected:
Batch mode should now print an error instead of a warning, and return a non-zero exit code

Actual:
The issue is still printed as a warning.

Digging into this, I determined the issue is with the handling of verilog-warn-fatal-internal which is set to nil when in batch mode by verilog-batch-error-wrapper. The wrapper is intended to add the error prefix to any errors, however because the verilog-warn-fatal-internal is set to nil, the unintended side effect is that verilog-warn-fatal has no effect when set in batch mode.

The most straightforward solution appears to be just removing the verilog-warn-fatal-internal non-nil condition. I'm not sure if this has any unintended side effects, but this does solve the issue for my purposes at least.

Before:

(defun verilog-warn-error (string &rest args)
  "Call `error' using STRING and optional ARGS.
If `verilog-warn-fatal' is non-nil, call `verilog-warn' instead."
  (apply (if (and verilog-warn-fatal verilog-warn-fatal-internal)
             #'error #'verilog-warn)
         string args))

After:

(defun verilog-warn-error (string &rest args)
  "Call `error' using STRING and optional ARGS.
If `verilog-warn-fatal' is non-nil, call `verilog-warn' instead."
  (apply (if (and verilog-warn-fatal)
             #'error #'verilog-warn)
         string args))

I believe with that fix if there are multiple problems then only one error will be shown, which will be annoying to usability.

Instead, probably the end of verilog-batch-error-wrapper should see if any warnings have been printed, and if so error out.

Might you make a github pull to try that?