Skip to content
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.

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

Reorder DFDL statement processing to match spec #1369

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jadams-tresys
Copy link
Contributor

We had been processing all asserts and discriminators before the group
content, but according to section 9.5.2 of the spec non-pattern asserts
and discriminators should be evaluated after the group content, even if
the group content fails to parse.

DFDL-1971

Josh Adams added 2 commits November 11, 2024 09:32
We had been processing all asserts and discriminators before the group
content, but according to section 9.5.2 of the spec non-pattern asserts
and discriminators should be evaluated after the group content, even if
the group content fails to parse.

DFDL-1971
@jadams-tresys
Copy link
Contributor Author

When testing I noticed that dfdl-gif and dfdl-jpeg2000 had some failures with this change. I suspect that these schemas might need a small update as the order of when discriminators and assert statements are evaluated are changed and these schemas seem to be relying on the old/incorrect behavior

Copy link
Contributor

@mbeckerle mbeckerle left a comment

Choose a reason for hiding this comment

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

Pretty sure this won't be correct as is.

More cases need tests. Where the body parses successfully but the discriminator fails, where the body fails and the discriminator fails, where the body fails, but the discriminator succeeds - and references elements in the partial body infoset (infoset as it existed at the time of the body failure)

dfdlLowPriorityStatementEvaluations ~ // setVariable and the rest of the Assert and Discriminator statements
groupLeftFraming ~ groupContentWithInitiatorTerminator ~ groupRightFraming ~ dfdlScopeEnd
setVariableEvaluations ~
groupLeftFraming ~ groupContentWithInitiatorTerminator ~ nonPatternEvaluations ~ groupRightFraming ~ dfdlScopeEnd
Copy link
Contributor

Choose a reason for hiding this comment

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

I think more is needed than this repositioning of the evaluation order.

The enclosing term combinator (for an element or a model group) may need to get the nonPatternEvaluations separately from the rest of the sequence of things to do. In fact, it might be necessary to isolate the nonPatternDiscriminatorEvaluations from the regular non-pattern asserts, as they get slightly different treatment.

So rather than this big long ~ separated sequence, you'll need the combinator to be passed this sequence up to, but not including the nonPatternDiscriminatorEvaluations, then pass the nonPatternDiscriminatorEvaluations, and then everything after those. The combinator then orchestrates evaluating the "body", then the nonPatternDiscriminatorEvaluations, that may set the discriminator true on the stack, and if so that changes the way the "body" failure will be handled.

This whole thing is kind of a trick to make it possible for discriminator/assert statements to look downward into the complexType/group on which they are expressed, instead of having to refer only backward to prior already parsed fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants