Fix separator not backtracked when complex-typed array element fails - #1716
Open
olabusayoT wants to merge 6 commits into
Open
olabusayoT wants to merge 6 commits into
olabusayoT wants to merge 6 commits into
Conversation
olabusayoT
force-pushed
the
daf-3094-absentrep-backtrack
branch
from
September 3, 2026 18:05
6f67018 to
8ae8dbb
Compare
For a maxOccurs-bounded occursCountKind="implicit" array whose element type is complex with no possible zero-length representation (e.g. every branch of a choice requires a distinct non-empty initiator), the speculative attempt at an occurrence beyond the real data would parse its preceding separator successfully, then fail the content entirely with zero bits consumed. anyTypeElementFailedParseAttemptStatus classified this as AbsentRep, whose contract is to retain the bit position (keep the separator consumed); that is correct for a genuinely empty representation, but wrong here since there is no occurrence at all. This let the separator leak into whatever sequence child comes next, causing spurious "separator not found" / "terminator not found" errors. Gate the AbsentRep classification on isEmptyRepZeroLength for complex types, so a structurally-impossible zero-length failure returns MissingItem instead, which forces the caller to fully backtrack the point of uncertainty including the separator. Added tests to exercise. DAFFODIL-3094
olabusayoT
force-pushed
the
daf-3094-absentrep-backtrack
branch
from
September 4, 2026 19:16
8ae8dbb to
1848c10
Compare
…backtracking The prior fix gated AbsentRep vs MissingItem on the static isEmptyRepZeroLength flag, but that only describes whether some successful parse of the type could be zero-length, not whether this particular failed attempt was genuinely empty. A choice whose real branch matches part of its content then fails nets zero bits once backed out, indistinguishable from a genuine empty match by bit position alone. Track this via pointOfUncertaintyResetCount, which counts resets that actually undo bit position movement, and a mark stack letting a sequence child's attempt check, via hadPointOfUncertaintyResetSinceMark, whether such a reset happened during it. Added tests for both cases. DAFFODIL-3094
olabusayoT
force-pushed
the
daf-3094-absentrep-backtrack
branch
from
September 8, 2026 14:20
dcdf762 to
4eeab17
Compare
…tion for complex array elements The Optional+zero-length branch of anyTypeElementFailedParseAttemptStatus unconditionally returned AbsentRep on a failed parse attempt. AbsentRep's caller-side handling backs out infoset side effects then re-jumps forward past whatever was consumed, retaining any separator found along the way. That is correct for the documented simple-type idiom (adjacent delimiters of an enclosing group indicating a genuinely empty positional slot), but wrong for complex types: per DFDL Spec 9.3.2.2, a complex type's empty or absent representation can only be established by a descent that returns successfully, never by one that fails. Per 9.2.4/9.2.6, Absent is the narrower, positionally-locatable subset of the broader Missing case; a failed complex-type descent belongs to Missing, not Absent. The prior fix (4eeab17) tried to patch this with a static isEmptyRepZeroLength check and, when that proved insufficient, a PoU-resolve-count runtime signal; both are reverted here in favor of an unconditional type check, gating AbsentRep to erd.isSimpleType and letting all complex-type failures fall through to the existing MissingItem/FailureUnspecified handling, which performs a genuine full backtrack. Also updates the emptyElementDetectionAssertion idiom across SepTests.tdml to the bare pattern-discriminator form used in production schemas. DAFFODIL-3094
Removes the erd.isSimpleType restriction added by 485fc8b (gating AbsentRep on a failed zero-length attempt to simple types only) and restores the original comment, making this method byte-identical to its state before any DAFFODIL-3094 work. The complex-type separator-retention problem this guarded against needs a different fix, built at the caller (SeparatedParseHelper's separator-handling code) instead of baking a type check into this shared classify method, which should stay untouched so its behavior is easy to reason about independently of caller-side verification logic. DAFFODIL-3094
… sequences Complex-typed elements determine zero length from bit-position bookkeeping alone, which nested backtracking makes unreliable. Adds a non-consuming scanner (ZeroLengthDelimiterScanner/ZeroLengthComplexTypeDelimitedParser) that checks for a genuine in-scope delimiter where such an element's content would begin, consulted on the failure path. A potentially-trailing occurrence can fail on real content, backtracking its whole attempt including its separator; if the next sibling then succeeds, that separator was silently reused. Adds a check to arrayCompleteChecks (previously only for separatorSuppressionPolicy='never') via a new priorSiblingResultOfTry that survives a repeating child's own inner loop, left Uninitialized for a multi-occurrence sibling whose final probe is expected to fail. separatorSuppressionPolicy='anyEmpty' also requires a zero-length occurrence to omit its separator; a matching check uses a new pstate.lastSeparatorWasFound, gated off occursCountKind 'parsed'/'stopValue'. Updated tests. DAFFODIL-3094
olabusayoT
force-pushed
the
daf-3094-absentrep-backtrack
branch
from
September 15, 2026 20:35
b499806 to
80f9b07
Compare
…ling occurrences under 'never', 'trailingEmpty', and 'trailingEmptyStrict'. Term.scala's couldBePotentiallyTrailing excluded any element carrying its own discriminator, so such elements fell back to plain Positional classification, silently skipping the separator-suppression check entirely regardless of policy. SequenceChild.scala now classifies these as PositionalNever/PositionalTrailingLax/PositionalTrailingStrict too, matching how potentially-trailing elements under the same policies are already handled, and restoring the intended enforcement for discriminator-bearing elements. Also removes the anyEmpty-specific arrayCompleteChecks check and its supporting plumbing (lastSeparatorWasFound/lastAttemptSeparatorWasFound, isAnyEmptySeparatorSuppressionPolicy), since it was based on a misreading of the spec's permissive "may be omitted" wording for 'anyEmpty', unlike the "must" wording for 'trailingEmptyStrict'. Adds test_sep_ssp_any_5 through _8 and updates never_/strict_ expectations to match corrected behavior. Also fixes NonPositional (anyEmpty) failed zero-length occurrences always classifying as MissingItem, discarding a separator that genuinely belonged to them; this broke real-world schemas mixing skipped optional elements with real separators. computeParseAttemptStatus now threads a separatorWasFound flag, and returns AbsentRep instead when another separator immediately follows (peeked via the new PState.probeNonDestructively), scoped to non-array occurrences since a repeating array's own loop can claim further separators itself. Also fixes an engine abort when a discriminator resolves the PoU on the same attempt that classifies AbsentRep, and corrects a wrong expected value in test_sep_ssp_any_7's fixture. DAFFODIL-3094
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
3 participants
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.
For a maxOccurs-bounded occursCountKind="implicit" array whose element type is complex with no possible zero-length representation (e.g. every branch of a choice requires a distinct non-empty initiator), the speculative attempt at an occurrence beyond the real data would parse its preceding separator successfully, then fail the content entirely with zero bits consumed. anyTypeElementFailedParseAttemptStatus classified this as AbsentRep, whose contract is to retain the bit position (keep the separator consumed); that is correct for a genuinely empty representation, but wrong here since there is no occurrence at all. This let the separator leak into whatever sequence child comes next, causing spurious "separator not found" / "terminator not found" errors.
Gate the AbsentRep classification on isEmptyRepZeroLength for complex types, so a structurally-impossible zero-length failure returns MissingItem instead, which forces the caller to fully backtrack the point of uncertainty including the separator.
DAFFODIL-3094