stevedlawrence commented on a change in pull request #262: Unordered sequences
URL: https://github.com/apache/incubator-daffodil/pull/262#discussion_r318124642
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/AlignedMixin.scala
##########
@@ -52,10 +52,20 @@ trait AlignedMixin extends GrammarMixin { self: Term =>
* will be properly aligned by where the prior thing left us positioned.
* Hence we are guaranteed to be properly aligned.
*/
- final def isKnownToBeAligned: Boolean = LV('isKnownToBeAligned) {
+ final lazy val isKnownToBeAligned: Boolean = LV('isKnownToBeAligned) {
if (!isRepresented) {
true
+ } else if (nearestEnclosingSequence.isDefined &&
!nearestEnclosingSequence.get.isOrdered) {
+ // Enclosed within an unordered sequence, all items in the seq
+ // must have a length that will leave the sequence aligned for
+ // isKnownToBeAligned to be true
+ val (priorSibs, optEnclosingParent) = potentialPriorTerms
+ priorSibs.forall(x => {
+ val contentAlignment =
AlignmentMultipleOf((x.elementSpecifiedLengthApprox +
x.trailingSkipApprox).nBits)
+ (contentAlignment % alignmentApprox) == 0
+ })
Review comment:
I'm not sure I completely understand the changes to this file.
It seems like the logic for ``isKnownToBeAligned`` doesn't need to change,
but instead we just need to change ``priorAlignment`` to take into account all
siblings intead of just prior siblings when things are unordered? And in fact,
I think ``potentialPriorTerms`` already does that, so I guess it's not clear to
me why this didn't already work.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
