From 7111fa5811d4396ca1648dfc62fc1997dd975087 Mon Sep 17 00:00:00 2001 From: olabusayoT <50379531+olabusayoT@users.noreply.github.com> Date: Mon, 24 Nov 2025 17:17:59 -0500 Subject: [PATCH 1/3] Add support for separator alignment in sequences - Implement logic to calculate prefix, infix, and postfix separator alignments and lengths in sequence terms. - Updated `priorAlignmentApprox` and `endingAlignmentApprox` to incorporate separator alignments and lengths when determining alignment. - Refactored `encodingLengthApprox` to reuse new separator-related alignment methods. - Added new test cases `test_sep_alignment_1`, `test_sep_alignment_2`, and `test_sep_alignment_3` for validation of separator alignments. - Updated TDML files with corresponding test data and schema definitions. DAFFODIL-2295 --- .../daffodil/core/grammar/AlignedMixin.scala | 83 ++++++++--- .../apache/daffodil/usertests/SepTests.tdml | 135 ++++++++++++++++-- .../daffodil/usertests/TestSepTests.scala | 5 + 3 files changed, 198 insertions(+), 25 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..48e66bc51c 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 @@ -21,12 +21,14 @@ import org.apache.daffodil.core.dsom.ElementBase import org.apache.daffodil.core.dsom.ModelGroup import org.apache.daffodil.core.dsom.QuasiElementDeclBase import org.apache.daffodil.core.dsom.Root +import org.apache.daffodil.core.dsom.SequenceTermBase import org.apache.daffodil.core.dsom.Term import org.apache.daffodil.lib.exceptions.Assert import org.apache.daffodil.lib.schema.annotation.props.gen.AlignmentKind import org.apache.daffodil.lib.schema.annotation.props.gen.AlignmentUnits import org.apache.daffodil.lib.schema.annotation.props.gen.LengthKind import org.apache.daffodil.lib.schema.annotation.props.gen.LengthUnits +import org.apache.daffodil.lib.schema.annotation.props.gen.SeparatorPosition import org.apache.daffodil.lib.util.Math case class AlignmentMultipleOf(nBits: Long) { @@ -134,11 +136,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]) { @@ -228,8 +233,53 @@ trait AlignedMixin extends GrammarMixin { self: Term => } }.value + private lazy val separatorPrefixMTAApprox = + this.optLexicalParent match { + case Some(s: SequenceTermBase) if s.hasSeparator => + import SeparatorPosition.* + s.separatorPosition match { + case Prefix | Infix => LengthMultipleOf(s.knownEncodingAlignmentInBits) + case Postfix => LengthMultipleOf(0) + } + case _ => LengthMultipleOf(0) + } + + private lazy val separatorPostfixMTAApprox = + this.optLexicalParent match { + case Some(s: SequenceTermBase) if s.hasSeparator => + import SeparatorPosition.* + s.separatorPosition match { + case Prefix | Infix => LengthMultipleOf(0) + case Postfix => LengthMultipleOf(s.knownEncodingAlignmentInBits) + } + case _ => LengthMultipleOf(0) + } + + private lazy val separatorLengthApprox = this.optLexicalParent match { + case Some(s: SequenceTermBase) if s.hasSeparator => + getEncodingLengthApprox(s) + 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) + } + } + private lazy val priorAlignmentWithLeadingSkipApprox: AlignmentMultipleOf = { - priorAlignmentApprox + leadingSkipApprox + val pawls = + priorAlignmentApprox + + separatorPrefixMTAApprox + + separatorLengthApprox + + leadingSkipApprox + pawls } protected lazy val contentStartAlignment: AlignmentMultipleOf = { @@ -242,15 +292,24 @@ trait AlignedMixin extends GrammarMixin { self: Term => } } + /** + * The postfix separator MTA/length needs to be added after the trailing skip + * + * TODO: DAFFODIL-3057 needs to consider terminator MTA/length before trailingSkip + */ protected lazy val endingAlignmentApprox: AlignmentMultipleOf = { this match { case eb: ElementBase => { - if (eb.isComplexType && eb.lengthKind == LengthKind.Implicit) { + val ea = if (eb.isComplexType && eb.lengthKind == LengthKind.Implicit) { eb.complexType.group.endingAlignmentApprox + trailingSkipApprox } else { // simple type or complex type with specified length contentStartAlignment + (elementSpecifiedLengthApprox + trailingSkipApprox) } + val endingAlignmentWithSeparatorApprox = ea + + separatorPostfixMTAApprox + + separatorLengthApprox + endingAlignmentWithSeparatorApprox } case mg: ModelGroup => { // @@ -329,15 +388,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/usertests/SepTests.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/usertests/SepTests.tdml index 0ab4ea916e..cf16171d92 100644 --- a/daffodil-test/src/test/resources/org/apache/daffodil/usertests/SepTests.tdml +++ b/daffodil-test/src/test/resources/org/apache/daffodil/usertests/SepTests.tdml @@ -143,14 +143,6 @@ separatorPosition="infix" emptyElementParsePolicy="treatAsEmpty"/> - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + aa/Bbb + + + + + aa + bb + + + + + + + + /Aaa/Bbb + + + + + aa + bb + + + + + + + + aa/Bbb/ + + + + + + aa + bb + + + + + + diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/usertests/TestSepTests.scala b/daffodil-test/src/test/scala/org/apache/daffodil/usertests/TestSepTests.scala index 3ffd036121..d99bad341e 100644 --- a/daffodil-test/src/test/scala/org/apache/daffodil/usertests/TestSepTests.scala +++ b/daffodil-test/src/test/scala/org/apache/daffodil/usertests/TestSepTests.scala @@ -55,4 +55,9 @@ class TestSepTests extends TdmlTests { // DAFFODIL-2791 @Test def test_treatAsAbsent_occursIndex = test + + // DAFFODIL-2295 + @Test def test_sep_alignment_1 = test + @Test def test_sep_alignment_2 = test + @Test def test_sep_alignment_3 = test } From 8323d7ea8b74d48be89de45fa26c5f19ed3c1e52 Mon Sep 17 00:00:00 2001 From: olabusayoT <50379531+olabusayoT@users.noreply.github.com> Date: Wed, 3 Dec 2025 15:55:20 -0500 Subject: [PATCH 2/3] fixup! Add support for separator alignment in sequences - Added `test_sep_alignment_4` to validate postfix separator alignment with complex nested sequences. - Refactored alignment logic, renaming `priorAlignmentWithLeadingSkipApprox` to `priorAlignmentWithLeftFramingApprox` and introducing `endingAlignmentWithRightFramingApprox` to handle right framing more intuitively. DAFFODIL-2295 --- .../daffodil/core/grammar/AlignedMixin.scala | 35 +++++++------ .../apache/daffodil/usertests/SepTests.tdml | 52 ++++++++++++++++++- .../daffodil/usertests/TestSepTests.scala | 2 + 3 files changed, 72 insertions(+), 17 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 48e66bc51c..95ec406d81 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 @@ -67,7 +67,7 @@ trait AlignedMixin extends GrammarMixin { self: Term => final lazy val isKnownToBeAligned: Boolean = LV(Symbol("isKnownToBeAligned")) { if (!isRepresented || (alignmentKindDefaulted == AlignmentKind.Manual)) true else { - val pa = priorAlignmentWithLeadingSkipApprox + val pa = priorAlignmentWithLeftFramingApprox val aa = alignmentApprox val res = (pa % aa) == 0 res @@ -87,7 +87,7 @@ trait AlignedMixin extends GrammarMixin { self: Term => else if (isKnownEncoding) { if (knownEncodingAlignmentInBits == 1) true - else if (priorAlignmentWithLeadingSkipApprox.nBits % knownEncodingAlignmentInBits == 0) + else if (priorAlignmentWithLeftFramingApprox.nBits % knownEncodingAlignmentInBits == 0) true else false @@ -206,7 +206,7 @@ trait AlignedMixin extends GrammarMixin { self: Term => // Return 0 here, unordered alignment will be handled by unorderedSequenceSelfAlignment AlignmentMultipleOf(0) } else { - ps.endingAlignmentApprox + ps.endingAlignmentWithRightFramingApprox } eaa } @@ -273,7 +273,7 @@ trait AlignedMixin extends GrammarMixin { self: Term => } } - private lazy val priorAlignmentWithLeadingSkipApprox: AlignmentMultipleOf = { + private lazy val priorAlignmentWithLeftFramingApprox: AlignmentMultipleOf = { val pawls = priorAlignmentApprox + separatorPrefixMTAApprox @@ -283,33 +283,24 @@ trait AlignedMixin extends GrammarMixin { self: Term => } protected lazy val contentStartAlignment: AlignmentMultipleOf = { - if (priorAlignmentWithLeadingSkipApprox % alignmentApprox == 0) { + if (priorAlignmentWithLeftFramingApprox % alignmentApprox == 0) { // alignment won't be needed, continue using prior alignment as start alignment - priorAlignmentWithLeadingSkipApprox + priorAlignmentWithLeftFramingApprox } else { // alignment will be needed, it will be forced to be aligned to alignmentApprox alignmentApprox } } - /** - * The postfix separator MTA/length needs to be added after the trailing skip - * - * TODO: DAFFODIL-3057 needs to consider terminator MTA/length before trailingSkip - */ protected lazy val endingAlignmentApprox: AlignmentMultipleOf = { this match { case eb: ElementBase => { - val ea = if (eb.isComplexType && eb.lengthKind == LengthKind.Implicit) { + if (eb.isComplexType && eb.lengthKind == LengthKind.Implicit) { eb.complexType.group.endingAlignmentApprox + trailingSkipApprox } else { // simple type or complex type with specified length contentStartAlignment + (elementSpecifiedLengthApprox + trailingSkipApprox) } - val endingAlignmentWithSeparatorApprox = ea - + separatorPostfixMTAApprox - + separatorLengthApprox - endingAlignmentWithSeparatorApprox } case mg: ModelGroup => { // @@ -350,6 +341,18 @@ trait AlignedMixin extends GrammarMixin { self: Term => } } + /** + * The postfix separator MTA/length needs to be added after the trailing skip + * + * TODO: DAFFODIL-3057 needs to consider terminator MTA/length before trailingSkip + */ + protected lazy val endingAlignmentWithRightFramingApprox: AlignmentMultipleOf = { + val res = endingAlignmentApprox + + separatorPostfixMTAApprox + + separatorLengthApprox + res + } + protected lazy val elementSpecifiedLengthApprox: LengthApprox = { this match { case eb: ElementBase => { diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/usertests/SepTests.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/usertests/SepTests.tdml index cf16171d92..61d08de6b9 100644 --- a/daffodil-test/src/test/resources/org/apache/daffodil/usertests/SepTests.tdml +++ b/daffodil-test/src/test/resources/org/apache/daffodil/usertests/SepTests.tdml @@ -730,7 +730,7 @@ - + @@ -752,6 +752,39 @@ + + + + + + + + + + + + + + + + + + + @@ -797,4 +830,21 @@ + + cc-Aaa/Bbb/-dd- + + + + + + cc + aa + bb + dd + + + + + + diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/usertests/TestSepTests.scala b/daffodil-test/src/test/scala/org/apache/daffodil/usertests/TestSepTests.scala index d99bad341e..9c27ba7261 100644 --- a/daffodil-test/src/test/scala/org/apache/daffodil/usertests/TestSepTests.scala +++ b/daffodil-test/src/test/scala/org/apache/daffodil/usertests/TestSepTests.scala @@ -60,4 +60,6 @@ class TestSepTests extends TdmlTests { @Test def test_sep_alignment_1 = test @Test def test_sep_alignment_2 = test @Test def test_sep_alignment_3 = test + + @Test def test_sep_alignment_4 = test } From 1b6f98dc643f43478a151142b22581967c0c2911 Mon Sep 17 00:00:00 2001 From: olabusayoT <50379531+olabusayoT@users.noreply.github.com> Date: Wed, 17 Dec 2025 13:31:38 -0500 Subject: [PATCH 3/3] fixup! MTA are alignments not lengths DAFFODIL-2295 --- .../daffodil/core/grammar/AlignedMixin.scala | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 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 95ec406d81..50722e1360 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 @@ -238,10 +238,10 @@ trait AlignedMixin extends GrammarMixin { self: Term => case Some(s: SequenceTermBase) if s.hasSeparator => import SeparatorPosition.* s.separatorPosition match { - case Prefix | Infix => LengthMultipleOf(s.knownEncodingAlignmentInBits) - case Postfix => LengthMultipleOf(0) + case Prefix | Infix => AlignmentMultipleOf(s.knownEncodingAlignmentInBits) + case Postfix => AlignmentMultipleOf(0) } - case _ => LengthMultipleOf(0) + case _ => AlignmentMultipleOf(0) } private lazy val separatorPostfixMTAApprox = @@ -249,10 +249,10 @@ trait AlignedMixin extends GrammarMixin { self: Term => case Some(s: SequenceTermBase) if s.hasSeparator => import SeparatorPosition.* s.separatorPosition match { - case Prefix | Infix => LengthMultipleOf(0) - case Postfix => LengthMultipleOf(s.knownEncodingAlignmentInBits) + case Prefix | Infix => AlignmentMultipleOf(0) + case Postfix => AlignmentMultipleOf(s.knownEncodingAlignmentInBits) } - case _ => LengthMultipleOf(0) + case _ => AlignmentMultipleOf(0) } private lazy val separatorLengthApprox = this.optLexicalParent match { @@ -275,8 +275,8 @@ trait AlignedMixin extends GrammarMixin { self: Term => private lazy val priorAlignmentWithLeftFramingApprox: AlignmentMultipleOf = { val pawls = - priorAlignmentApprox - + separatorPrefixMTAApprox + (priorAlignmentApprox + * separatorPrefixMTAApprox) + separatorLengthApprox + leadingSkipApprox pawls @@ -347,8 +347,8 @@ trait AlignedMixin extends GrammarMixin { self: Term => * TODO: DAFFODIL-3057 needs to consider terminator MTA/length before trailingSkip */ protected lazy val endingAlignmentWithRightFramingApprox: AlignmentMultipleOf = { - val res = endingAlignmentApprox - + separatorPostfixMTAApprox + val res = (endingAlignmentApprox + * separatorPostfixMTAApprox) + separatorLengthApprox res }