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

Reply via email to