Define isKnownNonZeroLength for delimiters - #1729
Open
aaronmbauman wants to merge 1 commit into
Open
aaronmbauman wants to merge 1 commit into
aaronmbauman wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
ModelGroup.hasFraminganswers whether a delimiter is written in the schema,not whether it occupies bits in the data stream. A
%ES;or%WSP*;delimitermatches zero-length data, so per DFDL spec section 9.2 it is not framing.
Because
hasFramingshort-circuitshasKnownRequiredSyntax, a group framed onlyby such a delimiter counts as definitely non-zero-length and is denied separator
suppression: parsing raises a spurious
Failed to find infix separator, andunparsing writes a trailing separator that does not belong.
The zero-length test now runs per literal in the delimiter list, and splits in two:
isKnownCanMatchEmptyStringfor the framing analysis, andhasZeroLengthEntityAlonefor the narrower restriction section 12.2 actually states.
Evidence
The same TDML suite run against
mainand this branch (TestSequenceGroupZLFraming):esInit_absent,esTerm_absent,wspInit_absent,esChoice_absent,esInitList,wspWspInit,esTermList,exprTermRealFailed to find infix separatoresInit_absent_unparse,wspWspInit_unparseoutput data length 2 for 'X,'Xctl_*,esInit_present,constTermReal, ...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-testagainst 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 lengthwas 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 aSchema 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
emptyInitiator8andemptyInitiator9.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 atcompile time.
AlignedMixinkeeps usinghasSeparator. A delimiterthat 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
isKnownNonZeroLengthfor delimiters so theterm-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:SeparatorCookerisDelimiterCookerNoES, so a constant%ES;/%WSP*;separator is already an SDE andnever 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 thecooker and then aborts in the delimiter DFA (
Assert.impossibleCase,dfa/Rules.scala:117) onmaintoo. I can file that if you want it tracked.