stevedlawrence commented on code in PR #1440:
URL: https://github.com/apache/daffodil/pull/1440#discussion_r1963500363
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/SeparatedSequenceParsers.scala:
##########
@@ -129,17 +129,17 @@ final class OrderedSeparatedSequenceParser(
spos: SeparatorPosition,
sep: Parser,
childrenArg: Vector[SequenceChildParser]
-) extends SequenceParserBase(rd, childrenArg, isOrdered = true) {
+) extends SequenceParserBase(rd, childrenArg.toArray, isOrdered = true) {
- override lazy val childProcessors = (sep +:
childrenArg.asInstanceOf[Seq[Parser]]).toVector
+ override def childProcessors = (sep +:
childrenArg.asInstanceOf[Seq[Parser]]).toVector
}
final class UnorderedSeparatedSequenceParser(
rd: SequenceRuntimeData,
spos: SeparatorPosition,
sep: Parser,
childrenArg: Vector[SequenceChildParser]
Review Comment:
Same here, this probably wants to be an Array.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/SeparatedSequenceParsers.scala:
##########
@@ -129,17 +129,17 @@ final class OrderedSeparatedSequenceParser(
spos: SeparatorPosition,
sep: Parser,
childrenArg: Vector[SequenceChildParser]
Review Comment:
Should we change the type of `childrenArg` to an Array? I believe
`childrenArg` will be serialized, and I think as a rule of thumb anything that
could fail deserialization due to an easy to make classpath issue (which
includes parsers) should use Array instead of Vector to ensure deserialization
diagnostics are reasonable.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/UnseparatedSequenceParsers.scala:
##########
@@ -80,9 +80,9 @@ class RepOrderedWithMinMaxUnseparatedSequenceChildParser(
class OrderedUnseparatedSequenceParser(
rd: SequenceRuntimeData,
childParsersArg: Vector[SequenceChildParser]
Review Comment:
`childParsersArg` and `choiceParser` in this file probably want to be Arrays
also.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/Grammar.scala:
##########
@@ -92,7 +92,7 @@ class SeqComp private (context: SchemaComponent, children:
Seq[Gram])
final override lazy val parser = {
Review Comment:
I did a grep and it looks like there's a handful of vals that were missed:
```
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/unparsers/Unparser.scala:
override val childProcessors = childUnparsers
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/ChoiceAndOtherVariousUnparsers.scala:
override val childProcessors = choiceBranchMap.childProcessors.toVector
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/StreamSplitterMixin.scala:
override val childProcessors: Vector[Processor] = Vector()
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SeparatedSequenceUnparsers.scala:
val childProcessors = Vector(childUnparser, sep)
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/UnseparatedSequenceUnparsers.scala:
val childProcessors = Vector(childUnparser)
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SuppressableSeparatorUnparser.scala:
override val childProcessors: Vector[Processor] = Vector(sepUnparser)
```
Some of those classes also look like the passed in parameters should be
changed from a Vector to an Array as well..
--
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]
