Discrepancy in warning/error message

[titan73]

Describe the bug

The fist message says it's an error but displayed as warning in logger and causes the subsequent error:

[WARNING ODB-0099] error: netlist component (I_digital_top) is not defined
[ERROR ODB-0421] DEF parser returns an error!

Expected Behavior

No discrepancy

Environment

Latest git.

To Reproduce

No need.

Relevant log output

No response

Screenshots

No response

Additional Context

No response

[maliberty]

We want to print all the issues, not just the first. Since error stops on the first we used warning and then error at the end. It is a bit odd. You could store up all the warnings and then just issue one big error.

[titan73]

I've look at the code and saw some similar warnings but with missing error marker "error:":

      ++_errors;
      _logger->warn(utl::ODB, 117, "PIN {} missing right bus character.", name);
      return;

I also see several if/else construct that display either a warning or an error depending on a "continue_on_errors" variable:

  if (res != 0) {
    if (!_continue_on_errors) {
      _logger->error(utl::ODB, 422, "DEF parser returns an error!");
    } else {
      _logger->warn(utl::ODB, 151, "DEF parser returns an error!");
    }
  }

Which creates the same messages with 2 different error numbers.

What about adding a bool argument "continue_on_error" or "fail" or "dont_fail" to the logger error method with a default value that triggers the exception. This would give something like that:

  if (res != 0) {
      _logger->error(utl::ODB, 422, "DEF parser returns an error!", _continue_on_errors);
  }

In the normal case where it fails as in the usual code: _logger->error(xxx, yyyy, "zzzz");

I can try to do a patch if agreed.

[maliberty]

The message can take arguments so you can't put it after the message (eg "bad {}", value). Putting it before the message string is possible but then you can't use a default argument which would be awkward at best. I don't see a viable path for this suggestion.

[titan73]

Ah yeah indeed. :s

[titan73]

This would require an additional variant function error_fail_if that has this argument for callers that want not to fail or fail conditionally.

[maliberty]

It starts to feel ugly with error having a conditional meaning. I think the problem originates in not really stopping on the first error. I think it is simpler to just error and stop or if --continue_on_errors then warn and don't stop. This is all pretty specific to the LEF/DEF parser. The Liberty and Verilog parsers stop on first errors afaik.