mbeckerle commented on a change in pull request #34: Daffodil 1884 bit order
URL: https://github.com/apache/incubator-daffodil/pull/34#discussion_r166787539
##########
File path:
daffodil-runtime1/src/main/scala/edu/illinois/ncsa/daffodil/processors/unparsers/Unparser.scala
##########
@@ -64,6 +64,10 @@ sealed trait Unparser
if (ustate.dataProc.isDefined) ustate.dataProc.get.before(ustate, this)
try {
unparse(ustate)
+ this.context match {
+ case trd: TermRuntimeData =>
ustate.dataOutputStream.setPriorBitOrder(ustate.bitOrder)
+ case _ => //ok
+ }
Review comment:
They are different. Parsing proceeds in order (ignoring backtracking) so
every primitive unparser that could leave one at a frag byte must get its bit
order, and it is checked, and checking sets the prior if it is changing.
We did have code at one time like this match-case in unparser in the parser
code, but I convinced myself by the above argument that we don't need it.
Unparsing some unparsers are suspendable, so there are a bunch of cases
here. Clearly calling this setter here is overkilling the situation. In theory
some places in the unparser code dealing with splitting/suspending are missing
proper management of the priorBitOrder. Finding those has been problematic. So
this is a temporary fix, until I can rationalize the setting of priorBitOrder
fully in the unparser.
I will insert comments here accordingly explaining that this is a fix, but
inefficient, and should be unnecessary.
No question that a design note is needed here explaining the invariants
here, and how it is supposed to work, all of which would be nice to capture
when writing the code the first time, but somehow never happens.
I am adding more comments to the code here and in other places explaining
what I can about how this works, and hopefully I'll find the issue as a part of
that process. If not, well at least all the tests we have are passing.
----------------------------------------------------------------
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
