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: us...@infra.apache.org With regards, Apache Git Services