From 4550ae6aff216b0a5052f0d8822a2c218ea25e3c Mon Sep 17 00:00:00 2001 From: olabusayoT <50379531+olabusayoT@users.noreply.github.com> Date: Tue, 9 Dec 2025 14:12:57 -0500 Subject: [PATCH 1/2] Update alignment to handle terminator alignment - Add support for including terminator MTA and length in `endingAlignmentApprox` calculations. - Introduce `terminatorMTAApprox` and `terminatorLengthApprox` for reusable terminator alignment logic. - Add new test cases (`test_sep_alignment_5`, `test_sep_alignment_6`) to validate terminator alignment in sequences. - Update comments DAFFODIL-3057 --- .../daffodil/core/grammar/AlignedMixin.scala | 81 +++++++++++----- .../section12/aligned_data/Aligned_Data.tdml | 92 +++++++++++++++++++ .../aligned_data/TestAlignedData.scala | 4 + 3 files changed, 156 insertions(+), 21 deletions(-) diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/AlignedMixin.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/AlignedMixin.scala index a6d52b8a5c..d06aacdb43 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/AlignedMixin.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/AlignedMixin.scala @@ -134,11 +134,14 @@ trait AlignedMixin extends GrammarMixin { self: Term => LengthExact(trailingSkipInBits) } - // FIXME: DAFFODIL-2295 - // Does not take into account that in a sequence, what may be prior may be a separator. - // The separator is text in some encoding, might not be the same as this term's encoding, and - // the alignment will be left where that text leaves it. - // + /** + * The priorAlignmentApprox doesn't need to take into account the separator + * alignment/length as that comes after the priorAlignmentApprox, but before + * the leadingSkip. + * + * TODO: DAFFODIL-3056 We need to consider the initiator MTA/length, + * as well as the prefix length + */ private lazy val priorAlignmentApprox: AlignmentMultipleOf = LV(Symbol("priorAlignmentApprox")) { if (this.isInstanceOf[Root] || this.isInstanceOf[QuasiElementDeclBase]) { @@ -232,6 +235,31 @@ trait AlignedMixin extends GrammarMixin { self: Term => priorAlignmentApprox + leadingSkipApprox } + private lazy val terminatorMTAApprox = + this match { + case t if t.hasTerminator => + LengthMultipleOf(t.knownEncodingAlignmentInBits) + case _ => LengthMultipleOf(0) + } + + private lazy val terminatorLengthApprox = this match { + case t if t.hasTerminator => + getEncodingLengthApprox(t) + case _ => LengthMultipleOf(0) + } + + private def getEncodingLengthApprox(t: Term) = { + if (t.isKnownEncoding) { + val dfdlCharset = t.charsetEv.optConstant.get + val fixedWidth = + if (dfdlCharset.maybeFixedWidth.isDefined) dfdlCharset.maybeFixedWidth.get else 8 + LengthMultipleOf(fixedWidth) + } else { + // runtime encoding, which must be a byte-length encoding + LengthMultipleOf(8) + } + } + protected lazy val contentStartAlignment: AlignmentMultipleOf = { if (priorAlignmentWithLeadingSkipApprox % alignmentApprox == 0) { // alignment won't be needed, continue using prior alignment as start alignment @@ -242,15 +270,25 @@ trait AlignedMixin extends GrammarMixin { self: Term => } } + /** + * Accounts for the terminator MTA/Length and trailing skip + */ protected lazy val endingAlignmentApprox: AlignmentMultipleOf = { this match { case eb: ElementBase => { - if (eb.isComplexType && eb.lengthKind == LengthKind.Implicit) { - eb.complexType.group.endingAlignmentApprox + trailingSkipApprox + val res = if (eb.isComplexType && eb.lengthKind == LengthKind.Implicit) { + eb.complexType.group.endingAlignmentApprox + + terminatorMTAApprox + + terminatorLengthApprox + + trailingSkipApprox } else { // simple type or complex type with specified length - contentStartAlignment + (elementSpecifiedLengthApprox + trailingSkipApprox) + contentStartAlignment + (elementSpecifiedLengthApprox + + terminatorMTAApprox + + terminatorLengthApprox + + trailingSkipApprox) } + res } case mg: ModelGroup => { // @@ -266,21 +304,30 @@ trait AlignedMixin extends GrammarMixin { self: Term => // it the actual last child. // val lceaa = lc.endingAlignmentApprox - val res = lceaa + trailingSkipApprox + val res = lceaa + + terminatorMTAApprox + + terminatorLengthApprox + + trailingSkipApprox res } val optApproxIfNoChildren = // // gather possibilities for this item itself // - if (couldBeLast) + if (couldBeLast) { // // if this model group could be last, then consider // if none of its content was present. // We'd just have the contentStart, nothing, and the trailing Skip. // - Some(contentStartAlignment + trailingSkipApprox) - else + val res = Some( + contentStartAlignment + + terminatorMTAApprox + + terminatorLengthApprox + + trailingSkipApprox + ) + res + } else // can't be last, no possibilities to gather. None val lastApproxes = lastApproxesConsideringChildren ++ optApproxIfNoChildren @@ -329,15 +376,7 @@ trait AlignedMixin extends GrammarMixin { self: Term => } private lazy val encodingLengthApprox: LengthApprox = { - if (isKnownEncoding) { - val dfdlCharset = charsetEv.optConstant.get - val fixedWidth = - if (dfdlCharset.maybeFixedWidth.isDefined) dfdlCharset.maybeFixedWidth.get else 8 - LengthMultipleOf(fixedWidth) - } else { - // runtime encoding, which must be a byte-length encoding - LengthMultipleOf(8) - } + getEncodingLengthApprox(this) } protected lazy val isKnownToBeByteAlignedAndByteLength: Boolean = { diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section12/aligned_data/Aligned_Data.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section12/aligned_data/Aligned_Data.tdml index 119c799854..c25818a934 100644 --- a/daffodil-test/src/test/resources/org/apache/daffodil/section12/aligned_data/Aligned_Data.tdml +++ b/daffodil-test/src/test/resources/org/apache/daffodil/section12/aligned_data/Aligned_Data.tdml @@ -602,6 +602,66 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + aaTBbb + + + + + + aa + bb + + + + + + + + aabbT2ee + + + + + + aa + bb + + ee + + + + + diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section12/aligned_data/TestAlignedData.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section12/aligned_data/TestAlignedData.scala index 23967c96a8..860415d7b1 100644 --- a/daffodil-test/src/test/scala/org/apache/daffodil/section12/aligned_data/TestAlignedData.scala +++ b/daffodil-test/src/test/scala/org/apache/daffodil/section12/aligned_data/TestAlignedData.scala @@ -186,6 +186,10 @@ class TestAlignedData extends TdmlTests { @Test def alignmentFillByteDefined = test @Test def separatorMTA_01 = test + + // DAFFODIL-3057 + @Test def test_term_alignment_1 = test + @Test def test_term_alignment_2 = test } class TestBinaryInput extends TdmlTests { From 38c50ec8f355feca164fa3251d457b61a5f94920 Mon Sep 17 00:00:00 2001 From: olabusayoT <50379531+olabusayoT@users.noreply.github.com> Date: Wed, 17 Dec 2025 14:30:04 -0500 Subject: [PATCH 2/2] fixup! Update alignment to handle terminator alignment - MTA are alignments not lengths - simplify pattern matching to if statement - refactor duplicate addition of terminator + trailing skip DAFFODIL-2295 --- .../daffodil/core/grammar/AlignedMixin.scala | 61 ++++++++----------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/AlignedMixin.scala b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/AlignedMixin.scala index d06aacdb43..256afd445b 100644 --- a/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/AlignedMixin.scala +++ b/daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/AlignedMixin.scala @@ -236,16 +236,16 @@ trait AlignedMixin extends GrammarMixin { self: Term => } private lazy val terminatorMTAApprox = - this match { - case t if t.hasTerminator => - LengthMultipleOf(t.knownEncodingAlignmentInBits) - case _ => LengthMultipleOf(0) + if (this.hasTerminator) { + AlignmentMultipleOf(this.knownEncodingAlignmentInBits) + } else { + AlignmentMultipleOf(0) } - private lazy val terminatorLengthApprox = this match { - case t if t.hasTerminator => - getEncodingLengthApprox(t) - case _ => LengthMultipleOf(0) + private lazy val terminatorLengthApprox = if (this.hasTerminator) { + getEncodingLengthApprox(this) + } else { + LengthMultipleOf(0) } private def getEncodingLengthApprox(t: Term) = { @@ -278,17 +278,14 @@ trait AlignedMixin extends GrammarMixin { self: Term => case eb: ElementBase => { val res = if (eb.isComplexType && eb.lengthKind == LengthKind.Implicit) { eb.complexType.group.endingAlignmentApprox - + terminatorMTAApprox - + terminatorLengthApprox - + trailingSkipApprox } else { // simple type or complex type with specified length - contentStartAlignment + (elementSpecifiedLengthApprox - + terminatorMTAApprox - + terminatorLengthApprox - + trailingSkipApprox) + contentStartAlignment + elementSpecifiedLengthApprox } - res + val cea = res * terminatorMTAApprox + + terminatorLengthApprox + + trailingSkipApprox + cea } case mg: ModelGroup => { // @@ -299,16 +296,9 @@ trait AlignedMixin extends GrammarMixin { self: Term => val (lastChildren, couldBeLast) = mg.potentialLastChildren val lastApproxesConsideringChildren: Seq[AlignmentMultipleOf] = lastChildren.map { lc => // - // for each possible last child, add its ending alignment - // to our trailing skip to get where it would leave off were - // it the actual last child. - // + // for each possible last child, gather its endingAlignmentApprox val lceaa = lc.endingAlignmentApprox - val res = lceaa - + terminatorMTAApprox - + terminatorLengthApprox - + trailingSkipApprox - res + lceaa } val optApproxIfNoChildren = // @@ -318,21 +308,24 @@ trait AlignedMixin extends GrammarMixin { self: Term => // // if this model group could be last, then consider // if none of its content was present. - // We'd just have the contentStart, nothing, and the trailing Skip. + // We'd just have the contentStart // - val res = Some( - contentStartAlignment - + terminatorMTAApprox - + terminatorLengthApprox - + trailingSkipApprox - ) + val res = Some(contentStartAlignment) res } else // can't be last, no possibilities to gather. None val lastApproxes = lastApproxesConsideringChildren ++ optApproxIfNoChildren - Assert.invariant(lastApproxes.nonEmpty) - val res = lastApproxes.reduce { _ * _ } + // take all the gathered possibilities and add the ending alignment + // to terminator MTA/length and our trailing skip to get where it would leave off were + // each the actual last child. + val lastApproxesFinal = lastApproxes.map { + _ * terminatorMTAApprox + + terminatorLengthApprox + + trailingSkipApprox + } + Assert.invariant(lastApproxesFinal.nonEmpty) + val res = lastApproxesFinal.reduce { _ * _ } res } }