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]

Reply via email to