[GitHub] [daffodil] stevedlawrence commented on a change in pull request #579: Fix unordered sequences with discriminated content

[GitBox <gi...@apache.org>]

stevedlawrence commented on a change in pull request #579:
URL: https://github.com/apache/daffodil/pull/579#discussion_r643400471



##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/SequenceParserBases.scala
##########
@@ -321,20 +344,19 @@ abstract class SequenceParserBase(
       }
 
       // We finished parsing one part of a sequence, which could either be an
-      // array, simple or complex. If the last child is different from when we
-      // started that means we must have added a new element and we can set it
-      // final and walk.
+      // array, simple or complex. We aren't sure if we actully added new
+      // element or not, but in case we did, mark the last node as final and
+      // try walk in case we could output the infoset.
+      //
+      // Note that in the case of unordered sequences we are blocked, and so

Review comment:
       Yeah, this comment needs to be clarified.
   
   We have an InfosetWalker that periodically walks the internal infoset data structure and outputs infoset events. But it outputs events only once it knows that an infoset element can't be backtracked and undone, since we can't "undo" events once they are created. There are a couple places where we set flags on infoset elements that say they are "final" and that the infoset can walk/output their event.
   
   The problem is that with unordered sequences the individual infoset elements might think they are final and change that flag, but because those elements are in an unordered sequence they might actually get reordered once the the unordered sequence completes. So even though the elements might claim they are final, they actually aren't.
   
   To deal with this, we put a "block" on the infoset walker right before we start parsing an unordered sequence so none of it's children can be walked. Once the unordered sequence is complete and any reordering is done, we "unblock" the walker.
   
   The terminolgy used for this blocking is "infosetWalkerBlockCount" which is essentially just a refcounter that each infoset item has that prevents, or "blocks", the walker from making moving past it. At the beginning of an unordered sequence parse, we increment this block count and at the end we decrement that block count.
   
   This comment is really just saying that we are about to tell the infoset walker to walk, but it's not actually going to do anything because the unordered sequence has been "blocked". I think we can wrap this walk in a `if (!isOrdered)` conditional to make it even more clear. But ill also update the comment to make it more clear what's going on.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org