stevedlawrence commented on code in PR #987:
URL: https://github.com/apache/daffodil/pull/987#discussion_r1912067974
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/Parser.scala:
##########
@@ -227,26 +227,34 @@ 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 optDiscrimParser = childParsers.collectFirst { case ae:
AssertExpressionEvaluationParser if (ae.discrim) => ae }
+ val nonDiscrimChildren = if (optDiscrimParser.isDefined)
childParsers.diff(Seq(optDiscrimParser.get)) else childParsers
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
+ var i = 0
+
+ // Handle all non assert/discriminator child parsers first
+ while ((i < nonDiscrimChildren.size) && (pstate.processorStatus eq
Success)) {
Review Comment:
It might be worth storing `nonDiscrimChildren.size` as a member val? I
assume it's a constant time function so calling it in the loop probably doesn't
matter, but `numChildParser` used to be a member val so there might have been a
reason for that?
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/Parser.scala:
##########
@@ -227,26 +227,34 @@ 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 optDiscrimParser = childParsers.collectFirst { case ae:
AssertExpressionEvaluationParser if (ae.discrim) => ae }
+ val nonDiscrimChildren = if (optDiscrimParser.isDefined)
childParsers.diff(Seq(optDiscrimParser.get)) else childParsers
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
+ var i = 0
+
+ // Handle all non assert/discriminator child parsers first
Review Comment:
This comment should say "all non discriminator child parsers", removing
"assert". This will evaluate asserts if they exist.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/Parser.scala:
##########
@@ -227,26 +227,34 @@ 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 optDiscrimParser = childParsers.collectFirst { case ae:
AssertExpressionEvaluationParser if (ae.discrim) => ae }
+ val nonDiscrimChildren = if (optDiscrimParser.isDefined)
childParsers.diff(Seq(optDiscrimParser.get)) else childParsers
Review Comment:
I think this could be simplified to
```scala
val nonDiscrimChildren = childParsers.diff(optDiscrimParser.toSeq)
```
--
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]
