stevedlawrence commented on a change in pull request #100: Moved
pushDiscriminator to fix separators bug.
URL: https://github.com/apache/incubator-daffodil/pull/100#discussion_r210110761
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SeparatedSequenceParsers.scala
##########
@@ -299,6 +299,18 @@ final class OrderedSeparatedSequenceParser(
val bitPosBeforeSeparator = pstate.bitPos0b
+ pstate.pushDiscriminator
Review comment:
If we pushing the discriminator here, it means that if a separator parser
fails and processFailedSeparatorsWithPoU is called, the condition that checks
pstate.discriminator == true will never pass and Failed_WithDiscriminatorSet
will never be used (at least because of a missing separator), I think? So it
seems like that condition is never going to be hit?
If so, maybe the right fix is to put the pushDiscriminator back where it
was, and just remove the pstate.discriminitor == true check so that
processFailedSeparatorsWithPoU always returns Failed_SpeculativeParse if a
separator parser fails? Or is there some more complexity related to separators
and discriminators that is missing?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services