stevedlawrence commented on code in PR #1122:
URL: https://github.com/apache/daffodil/pull/1122#discussion_r1409351479
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/compiler/Compiler.scala:
##########
@@ -92,13 +89,10 @@ final class ProcessorFactory private (
validateDFDLSchemas,
checkAllTopLevel,
tunables,
- Some(sset),
Review Comment:
> I'd suggest that pf.getDiagnostics throw an assertion exception to ensure
such a programming error gets fixed, instead of calling isError before
returning the diagnostics or simply returning no diagnostics.
Doesn't seem unreasonable to me. We would have to add a boolean var since we
can't currently tell if a user calls isError or not, but that's not too big of
a deal.
There is maybe an argument that if a user calls `getDiagnostics` then they
aren't planning on calling any more `withXYZ` functions and we should just call
`isError` for them so they get the right result. This would mean calling
`isError` or `getDiagnostics` will trigger work.
Regardless, we should definitely do one of these. There were two instances
in our own codebase where we did this wrong, so it's not crazy think there
might be other places that do it and we don't want getDiagnostics to silently
return nothing. I don't feel strongly about which we choose. Calling `isError`
in `getDiagnostics` is maybe less likely to break things.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
