Define isKnownNonZeroLength for delimiters by aaronmbauman · Pull Request #1729 · apache/daffodil

aaronmbauman · 2026-09-21T00:48:59Z

What

ModelGroup.hasFraming answers whether a delimiter is written in the schema,
not whether it occupies bits in the data stream. A %ES; or %WSP*; delimiter
matches zero-length data, so per DFDL spec section 9.2 it is not framing.

Because hasFraming short-circuits hasKnownRequiredSyntax, a group framed only
by such a delimiter counts as definitely non-zero-length and is denied separator
suppression: parsing raises a spurious Failed to find infix separator, and
unparsing writes a trailing separator that does not belong.

The zero-length test now runs per literal in the delimiter list, and splits in two:
isKnownCanMatchEmptyString for the framing analysis, and hasZeroLengthEntityAlone
for the narrower restriction section 12.2 actually states.

Evidence

The same TDML suite run against main and this branch (TestSequenceGroupZLFraming):

	main	this branch
parse: `esInit_absent`, `esTerm_absent`, `wspInit_absent`, `esChoice_absent`, `esInitList`, `wspWspInit`, `esTermList`, `exprTermReal`	8 fail, `Failed to find infix separator`	pass
unparse: `esInit_absent_unparse`, `wspWspInit_unparse`	`output data length 2 for 'X,'`	writes `X`
controls: `ctl_*`, `esInit_present`, `constTermReal`, ...	pass	pass

Every case has a control that differs only in the delimiter on the inner model
group, so a control failure means the harness is broken rather than Daffodil.

Full daffodil-test against this commit: 4725 tests, 0 failures (4724 passed,
1 skipped, 115 ignored), on Ubuntu 22.04 / JDK 17 / Scala 3.3.7.

Notable for reviewers

checkModelGroupZL: it asserts that a model group which parsed zero length
was analyzed as possibly zero length. That could not hold while a %ES;
delimiter counted as framing, so the assert was commented out in 53d6b4f
(2018). It is enabled again.
dfdl:initiatedContent='yes': an initiator list such as "P %ES;" is now a
Schema Definition Error, as section 12.2 requires. Such schemas used to compile
and parse, so the commit message carries a Deprecation/Compatibility note with
a migration path, covered by possibleZeroLenEntityWithInitiatedContentNo and nonZeroLenInitiatorWithInitiatedContentYes.
Deliberately permissive: for an expression-valued delimiter we answer "not
known to occupy bits", which accepts input previously rejected. That is what
mil-std-2045 needs. It cannot be narrowed statically, since an expression
returning %ES; and one returning a real delimiter are indistinguishable at
compile time.
Alignment untouched: AlignedMixin keeps using hasSeparator. A delimiter
that can match zero length can also match non-zero length, so asserting zero
length there would risk silent misalignment.

On separators

The issue's requirement is to define isKnownNonZeroLength for delimiters so the
term-level calculations are correct, which this PR does for initiators and
terminators. It also notes the predicate can be false for separators where
lengthKind is not 'delimited'. I tried that: SeparatorCooker is
DelimiterCookerNoES, so a constant %ES;/%WSP*; separator is already an SDE and
never reaches the framing analysis, and wiring the same predicate into the separator
paths changed no observable behavior across four schema shapes, so I backed it out
rather than ship an untestable change.

Also found while testing, unrelated: dfdl:separator="%WSP*;%WSP*;" passes the
cooker and then aborts in the delimiter DFA (Assert.impossibleCase,
dfa/Rules.scala:117) on main too. I can file that if you want it tracked.

aaronmbauman · 2026-10-02T13:48:33Z

@stevedlawrence or @olabusayoT would appreciate a review of this

stevedlawrence · 2026-10-02T16:21:16Z

@aaronmbauman, yes, I plan to get to it next week. Thanks for the PR!

olabusayoT

Great work...some changes for readability and a comment correctness check

olabusayoT · 2026-10-06T17:43:51Z

      </xs:complexType>
    </xs:element>

+    <xs:element name="entityAloneInList">


maybe these elements should be prefixed with possibleZeroLenEntity instead of just entity

also this test elem should mention initatedContent so maybe possibleZeroLenEntityWithInitiatedContentYes (long but clear)

also also they should probably be in a different schema instead of emptyInitiator as they are not empty

Agreed, the old names didn't say much. They now start with possibleZeroLenEntity and say which initiatedContent value they use. I also pulled them out into their own possibleZeroLenEntityInitiator schema, since you're right that they aren't empty initiators.

olabusayoT · 2026-10-06T17:44:39Z

+      </xs:complexType>
+    </xs:element>
+
+    <xs:element name="entitiesNotAlone">


maybe renamed to multiplePossibleNonZeroEntities

Went with multiplePossibleZeroLenEntitiesWithInitiatedContentYes. I think you meant ZeroLen here, since %WSP*;%WSP*; can match nothing, but let me know if not.

olabusayoT · 2026-10-06T17:49:20Z

+      </xs:complexType>
+    </xs:element>
+
+    <xs:element name="migrateToInitiatedContentNo">


I don't understand the migrate prefix, should be possibleZeroLenEntityWithInitiatedContentNo or something like that

Fair, "migrate" only made sense if you'd read the commit message. It's possibleZeroLenEntityWithInitiatedContentNo now.

olabusayoT · 2026-10-06T17:49:54Z

+      </xs:complexType>
+    </xs:element>
+
+    <xs:element name="migrateDropZeroLengthAlternative">


change naming of this as well

Renamed to nonZeroLenInitiatorWithInitiatedContentYes.

olabusayoT · 2026-10-06T19:03:26Z

    </tdml:errors>
  </tdml:parserTestCase>
+
+  <tdml:parserTestCase name="emptyInitiator5"


same with the test names

The test names match the element names now.

olabusayoT · 2026-10-06T19:19:30Z

-      // Bug DAFFODIL-2132 is why isModelGroupRepNonZeroLength is incorrect in the above case.
-      //
-      // Assert.invariant(isModelGroupRepPossiblyZeroLength && !isModelGroupRepNonZeroLength)
+      Assert.invariant(isModelGroupRepPossiblyZeroLength && !isModelGroupRepNonZeroLength)


I didn't see any changes to isModelGroupRepPossiblyZeroLength, so the comment there probably reads incorrectly

You're right, that doc still described the old behavior. I rewrote it to match what SequenceChild actually computes.

Also sequenceChild.scala has a comment referencing the 2132 bug, I'd do a search just to make sure all comments are taken care of/updated

The DAFFODIL-2132 block in SequenceChild.scala was removed in this PR; it shows as deleted lines inside isModelGroupRepPossiblyZeroLength. Two other DAFFODIL-2132 comments were also removed: the FIXME on hasFraming in ModelGroup.scala, and the note on the commented-out assert in checkModelGroupZL, which is now enabled again. I searched the repo for other references: the only remaining DAFFODIL-2132 mentions are in the new SequenceGroupZLFraming tests, and I didn't find any other comments that still describe the old behavior.

ModelGroup.hasFraming answers whether a delimiter is written in the schema, not whether it occupies bits in the data stream. DFDL spec section 9.2 defines framing as the parts of the data stream "which are present and may be necessary to determine the length or position of the content", so a %ES; or %WSP*; delimiter, which matches zero-length data, is not framing. hasFraming short-circuits hasKnownRequiredSyntax, so a group framed only by one of those counts as definitely non-zero-length and is denied separator suppression: parsing raises a spurious "Failed to find infix separator" error, and unparsing writes a separator that does not belong. The zero-length test now runs per literal in the delimiter list, and splits in two. The framing analysis asks whether a literal can match zero-length data, which covers "X %ES;" and "%WSP*;%WSP*;" alike. The dfdl:initiatedContent check asks the narrower question section 12.2 asks, whether an entity appears "alone as one of the string literals in the list", so it diagnoses only what the spec restricts and leaves the rest to the runtime check in DelimiterParsers. Those same predicates now serve LocalElementMixin.couldBeMissing, which no longer analyzes delimiters by hand. A delimiter from an expression is not analyzed; the unknown case counts as zero-length matching, which is what mil-std-2045 needs. This covers dfdl:initiator and dfdl:terminator. The issue notes that the predicate can also be false for separators where lengthKind is not 'delimited', but a constant %ES; or %WSP*; separator is already a Schema Definition Error from the delimiter cooker, and giving the separator the same predicate changed no observable behavior. Not addressed here. checkModelGroupZL asserts that a model group which parsed zero length was analyzed as possibly zero length. That could not hold while a %ES; delimiter counted as framing, so the assert has been commented out since 2018. It is enabled again. Deprecation/Compatibility: - A schema with dfdl:initiatedContent='yes' whose dfdl:initiator lists %ES; or %WSP*; as one of its alternatives, such as "P %ES;", is now rejected with a Schema Definition Error, as the DFDL spec requires. Such schemas used to compile and parse. To migrate, set dfdl:initiatedContent='no', or drop the zero-length alternative from the initiator. - A sequence or choice group whose only initiator or terminator matches zero-length data, such as %ES; or %WSP*;, no longer counts as occupying space in the data. When such a group is last in a sequence, its separator may now be omitted on parse, and is no longer written on unparse. Data that failed with "Failed to find infix separator" now parses, and unparsed output no longer carries that trailing separator, so files compared against previously generated output will differ. DAFFODIL-2132

olabusayoT

+1 with fixes; some more comment updates

olabusayoT · 2026-10-07T18:47:21Z

-      // Bug DAFFODIL-2132 is why isModelGroupRepNonZeroLength is incorrect in the above case.
-      //
-      // Assert.invariant(isModelGroupRepPossiblyZeroLength && !isModelGroupRepNonZeroLength)
+      Assert.invariant(isModelGroupRepPossiblyZeroLength && !isModelGroupRepNonZeroLength)


Also sequenceChild.scala has a comment referencing the 2132 bug, I'd do a search just to make sure all comments are taken care of/updated

olabusayoT reviewed Oct 6, 2026

View reviewed changes

aaronmbauman force-pushed the DAFFODIL-2132 branch from 03bb776 to 6830c4f Compare October 6, 2026 22:35

olabusayoT approved these changes Oct 7, 2026

View reviewed changes

This comment was marked as duplicate.

Sign in to view

Conversation

aaronmbauman commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Evidence

Notable for reviewers

On separators

Uh oh!

aaronmbauman commented Oct 2, 2026

Uh oh!

stevedlawrence commented Oct 2, 2026

Uh oh!

olabusayoT left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olabusayoT left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as duplicate.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aaronmbauman commented Sep 21, 2026 •

edited

Loading