stevedlawrence commented on code in PR #1610:
URL: https://github.com/apache/daffodil/pull/1610#discussion_r2690954225
##########
daffodil-core/src/main/scala/org/apache/daffodil/runtime1/processors/DataProcessor.scala:
##########
@@ -190,7 +190,7 @@ class DataProcessor(
copy(variableMap = newVariableMap)
}
- override def isError = false
+ override def isError = ssrd == null || ssrd.parser == null || ssrd.unparser
== null
Review Comment:
Can this stay `isError = false`? I think it's impossible for a DataProcessor
to actually have an error since we check everything during processor factory.
If `ProcessorFactory.isError == true`, then calling
`ProcessorFactory.onPath()` should throw an usage exception and we should never
get a DataProcessor.
If `ProcessorFactory.isError == false`, then I *think* it's impossible to
create a DataProcessor since onPath doesn't do any new logic that could lead to
any errors. The requiredEvaluates at the tope of SchemaSetRuntime1Mixin ensures
eveything (e.g. parser, unparser) are evaluated when the SchemaSet is created
in the `ProcessorFactory.compile()`
Related, I wonder if we should change the onPath function to this:
```scala
Assert.invariant(!root.isError)
SchemaSetRuntimeData(
parser,
unparser,
...
)
```
And remove the !root.isError check that can create null parsers/unparser?
This makes it clear it's impossible for SchemaSetRuntimeData to get a null
parser/unparser, and that it's impossible for a DataProcessor to have an error.
--
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]