stevedlawrence commented on code in PR #987:
URL: https://github.com/apache/daffodil/pull/987#discussion_r1911338867
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/Parser.scala:
##########
@@ -227,26 +227,47 @@ abstract class CombinatorParser(override val context:
RuntimeData)
extends Parser
with CombinatorProcessor
-final class SeqCompParser(context: RuntimeData, val childParsers:
Vector[Parser])
- extends CombinatorParser(context) {
+final class SeqCompParser(
+ context: RuntimeData,
+ val childParsers: Vector[Parser],
+) extends CombinatorParser(context) {
override lazy val runtimeDependencies = Vector()
override def childProcessors = childParsers
override def nom = "seq"
- val numChildParsers = childParsers.size
+ val assertDiscrimExpressions = childParsers.collect { case ae:
AssertExpressionEvaluationParser => ae }
+ val discrimExpressions = assertDiscrimExpressions.filter { _.discrim }
+ val assertExpressions = assertDiscrimExpressions.filterNot { _.discrim }
+ val nonAssertChildren = childParsers.diff(assertExpressions)
def parse(pstate: PState): Unit = {
- var i: Int = 0
- while (i < numChildParsers) {
- val parser = childParsers(i)
- parser.parse1(pstate)
- if (pstate.processorStatus ne Success)
- return
+ var i = 0
+
+ // Handle all non assert/discriminator child parsers first
+ while ((i < nonAssertChildren.size) && (pstate.processorStatus eq
Success)) {
+ nonAssertChildren(i).parse1(pstate)
+ i += 1
+ }
+
+ // Handle all discriminator expressions statements after sequence body
+ i = 0
+ while (i < discrimExpressions.size) {
Review Comment:
If that's true we can simplify the logic a bit and remove the filtering of
asserts. We just need something like
```scala
val optDiscrimParser = ... // find an optional discrim parser
val nonDiscrimParsers = ... // remove discrim parsers if it exists
def parser() {
...
// call parse() for all nonDiscrimParsers
// call parse() for optDiscrimParser if set
...
}
```
If nonDiscrimParsers contains any asserts, then optDiscrimParser should be
None and the order is correct.
If nonDiscrimParsers does not contain asserts, then otpDiscrimParser will
always be evaluated (if it exists), and we don't have to worry about evaluating
assert parsers after the discrim because they shouldn't exist.
Should we also enforce the exclusivity as part of this PR, feels important
to ensuring the right behavior of assert/discrim evaluation.
--
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]
