mbeckerle commented on code in PR #987:
URL: https://github.com/apache/daffodil/pull/987#discussion_r2310879020
##########
daffodil-core/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/Parser.scala:
##########
@@ -227,26 +227,25 @@ abstract class CombinatorParser(override val context:
RuntimeData)
extends Parser
with CombinatorProcessor
-final class SeqCompParser(context: RuntimeData, val childParsers:
Array[Parser])
- extends CombinatorParser(context) {
+final class SeqCompParser(
+ context: RuntimeData,
+ val childParsers: Array[Parser],
+ testAssert: Array[Parser]
+) extends CombinatorParser(context) {
override def runtimeDependencies = Vector()
override def childProcessors = childParsers.toVector
override def nom = "seq"
- val numChildParsers = childParsers.size
-
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
- i += 1
+ childParsers.foreach { cp =>
+ if (pstate.processorStatus.isSuccess) cp.parse1(pstate)
+ else cp match {
+ case ae: AssertExpressionEvaluationParser if (ae.discrim) =>
pstate.withTempSuccess(ae.parse1)
+ case _ => // processorStatus is Failure, do not execute non-discim
child parsers
+ }
Review Comment:
> One potential issue with this approach is we don't short circuit failures.
For example, say there are 10000 parsers associated with this sequence and the
first one fails--we now have to iterate over all the remaining parsers and
ignore them just in case there is a discriminator at the end. Ideally on error
this would skip to the discriminator parser, evaluate that (assuming it hasn't
already been evaluated), and then end.
This is not right. In a given sequence, a discriminator expressed lexically
at the start of that sequence, that is, on that sequence's annotation, is to be
evaluated logically at the end of the sequence. But further, the discriminator
is supposed to be evaluated even if the sequence fails somewhere.
But note: This does *not* apply recursively. A discriminator expressed on or
inside one of the children of this sequence, does NOT count for this. Only a
discriminator expressed on that sequence's annotation point (it could come from
a group reference, or the sequence group itself), is evaluated even in the case
of failure.
So there is no walk of the 1000+ sequence children. This is a local action
by the sequence combinator which just has to evaluate the discriminator on the
unwind of the parse, even if the parse failed.
Note also, this applies only to discriminators with testKind="expression".
--
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]
