Skip to content

Update alignment to handle terminator alignment #1601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
olabusayoT wants to merge 2 commits into from
Closed

Update alignment to handle terminator alignment #1601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4550ae6
Update alignment to handle terminator alignment
olabusayoT Dec 9, 2025
38c50ec
fixup! Update alignment to handle terminator alignment
olabusayoT Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 61 additions & 29 deletions daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/AlignedMixin.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand Down Expand Up @@ -232,6 +235,31 @@ trait AlignedMixin extends GrammarMixin { self: Term =>
priorAlignmentApprox + leadingSkipApprox
}

private lazy val terminatorMTAApprox =
if (this.hasTerminator) {
AlignmentMultipleOf(this.knownEncodingAlignmentInBits)
} else {
AlignmentMultipleOf(0)
}

private lazy val terminatorLengthApprox = if (this.hasTerminator) {
getEncodingLengthApprox(this)
} else {
LengthMultipleOf(0)
}
Copy link
Member

@stevedlawrence stevedlawrence Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, can this be simplified to an if-statement?


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
Expand All @@ -242,15 +270,22 @@ 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
} else {
// simple type or complex type with specified length
contentStartAlignment + (elementSpecifiedLengthApprox + trailingSkipApprox)
contentStartAlignment + elementSpecifiedLengthApprox
}
val cea = res * terminatorMTAApprox
+ terminatorLengthApprox
+ trailingSkipApprox
cea
}
case mg: ModelGroup => {
//
Expand All @@ -261,31 +296,36 @@ 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 + trailingSkipApprox
res
lceaa
}
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.
// We'd just have the contentStart
//
Some(contentStartAlignment + trailingSkipApprox)
else
val res = Some(contentStartAlignment)
res
} else
Copy link
Member

@stevedlawrence stevedlawrence Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding the same three fields to a bunch of places in the various conditions can we do something like

val contentEndAlignment = ... //existing logic
val res =
  contentEndAlignment
    + terminatorMTAAPprox
    + terminatorLengthApprox
    + trailingSkipApprox

I'm not sure if contentEndAlignment is the right name, but something along those lines?

// 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
}
}
Expand Down Expand Up @@ -329,15 +369,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 = {
Expand Down
92 changes: 92 additions & 0 deletions ...odil-test/src/test/resources/org/apache/daffodil/section12/aligned_data/Aligned_Data.tdml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,66 @@
</xs:complexType>
</xs:element>
</tdml:defineSchema>

<tdml:defineSchema name="s2" elementFormDefault="unqualified">

<xs:include schemaLocation="/org/apache/daffodil/xsd/DFDLGeneralFormatPortable.dfdl.xsd"/>
<dfdl:format
ref="ex:GeneralFormat"
representation="text"
lengthKind="delimited"
separatorPosition="infix"
lengthUnits="bits"
alignmentUnits="bits"
/>

<xs:element name="postfix_term">
<xs:complexType>
<xs:sequence>
<xs:element name="e1" dfdl:lengthKind="implicit"
dfdl:fillByte="E">
<xs:complexType>
<xs:sequence>
<xs:element name="a" type="xs:string"
dfdl:length="16"
dfdl:lengthKind="explicit" dfdl:alignment="16"
dfdl:fillByte="A" dfdl:terminator="T"/>
<xs:element name="b" type="xs:string"
dfdl:length="16"
dfdl:lengthKind="explicit" dfdl:alignment="16"
dfdl:fillByte="B"/>
</xs:sequence>
</xs:complexType>
</xs:element>
</xs:sequence>
</xs:complexType>
</xs:element>

<xs:element name="postfix_term_2">
<xs:complexType>
<xs:sequence>
<xs:element name="e1" dfdl:lengthKind="implicit"
dfdl:fillByte="1" dfdl:terminator="T">
<xs:complexType>
<xs:sequence>
<xs:element name="a" type="xs:string"
dfdl:length="16"
dfdl:lengthKind="explicit" dfdl:alignment="16"
dfdl:fillByte="A"/>
<xs:element name="b" type="xs:string"
dfdl:length="16"
dfdl:lengthKind="explicit" dfdl:alignment="16"
dfdl:fillByte="B"/>
</xs:sequence>
</xs:complexType>
</xs:element>
<xs:element name="e2" type="xs:string"
dfdl:length="16" dfdl:fillByte="2"
dfdl:lengthKind="explicit" dfdl:alignment="16"/>
</xs:sequence>
</xs:complexType>
</xs:element>
</tdml:defineSchema>

<!--
Test Name: impAlignmentHexBinary
Expand Down Expand Up @@ -3909,5 +3969,37 @@
<tdml:documentPart type="bits">01</tdml:documentPart> <!-- base-4 "1" string (no mta fill bits)-->
</tdml:document>
</tdml:unparserTestCase>

<tdml:unparserTestCase name="test_term_alignment_1" root="postfix_term" model="s2" roundTrip="onePass">
<tdml:document>aaTBbb</tdml:document>

<tdml:infoset>
<tdml:dfdlInfoset>
<ex:postfix_term xmlns:ex="http://example.com">
<e1>
<a>aa</a>
<b>bb</b>
</e1>
</ex:postfix_term>
</tdml:dfdlInfoset>
</tdml:infoset>
</tdml:unparserTestCase>

<tdml:unparserTestCase name="test_term_alignment_2" root="postfix_term_2" model="s2" roundTrip="onePass">
<tdml:document>aabbT2ee</tdml:document>

<tdml:infoset>
<tdml:dfdlInfoset>
<ex:postfix_term_2 xmlns:ex="http://example.com">
<e1>
<a>aa</a>
<b>bb</b>
</e1>
<e2>ee</e2>
</ex:postfix_term_2>
</tdml:dfdlInfoset>
</tdml:infoset>
</tdml:unparserTestCase>


</tdml:testSuite>
4 changes: 4 additions & 0 deletions ...odil-test/src/test/scala/org/apache/daffodil/section12/aligned_data/TestAlignedData.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading