The-OpenROAD-Project
Discrepancy in warning/error message
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
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.
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.
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.
This would require an additional variant function error_fail_if that has this argument for callers that want not to fail or fail conditionally.
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.