Skip to content

Add support for dfdl:lengthKind="endOfParent" #1652

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

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

Add support for dfdl:lengthKind="endOfParent" #1652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e7bd04
Add support for `dfdl:lengthKind="endOfParent"`
olabusayoT Apr 3, 2026
df8100e
fixup!
olabusayoT May 7, 2026
4a974ba
fixup! fixup! fixup! fixup! fixup! fixup! fixup!
olabusayoT May 26, 2026
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
2 changes: 1 addition & 1 deletion daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ChoiceGroup.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ abstract class ChoiceTermBase(
* Open issues:
* 1) Is alignment or leading/trailing skip to be considered syntax. Alignment might not be there.
* 2) What about an empty sequence that only carries statement annotations such as dfdl:assert or
* dfdl:setVariable
* dfdl:setVariable
*
* This latter need to be allowed, because while they do not have known required syntax they do
* have to be executed for side-effect.
Expand Down
6 changes: 2 additions & 4 deletions daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/LocalElementMixin.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,8 @@ trait LocalElementMixin extends ParticleMixin with LocalElementGrammarMixin {
else if (representation =:= Representation.Binary) true
else false
}
case LengthKind.EndOfParent if isComplexType =>
notYetImplemented("lengthKind='endOfParent' for complex type")
case LengthKind.EndOfParent =>
notYetImplemented("lengthKind='endOfParent' for simple type")
// we can rarely statically know if an endOfParent element must have non-zero length
case LengthKind.EndOfParent => false
}
res
}.value
Expand Down
2 changes: 2 additions & 0 deletions daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SchemaSet.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import org.apache.daffodil.lib.iapi.Diagnostic
import org.apache.daffodil.lib.iapi.UnitTestSchemaSource
import org.apache.daffodil.lib.oolag.OOLAG
import org.apache.daffodil.lib.schema.annotation.props.LookupLocation
import org.apache.daffodil.lib.schema.annotation.props.gen.LengthUnits
import org.apache.daffodil.lib.util.TransitiveClosure
import org.apache.daffodil.lib.xml.*

Expand Down Expand Up @@ -116,6 +117,7 @@ final class SchemaSet private (

requiredEvaluationsAlways(root)
requiredEvaluationsAlways(checkForDuplicateTopLevels())
requiredEvaluationsAlways(root.checkEndOfParentRestrictions(Some(LengthUnits.Characters)))
Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence May 26, 2026

Choose a reason for hiding this comment

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

Is it possible to put this on the Root class? It feels like that' the right place for this check and any checks that algorithmically need to start from the root like this one.


lazy val resolver = DFDLCatalogResolver.get

Expand Down
1 change: 0 additions & 1 deletion daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SequenceGroup.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ abstract class SequenceGroupTermBase(xml: Node, lexicalParent: SchemaComponent,
case SequenceKind.Ordered => true
case SequenceKind.Unordered => false
}

}

/**
Expand Down
41 changes: 38 additions & 3 deletions daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/Term.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ import org.apache.daffodil.lib.iapi.WarnID
import org.apache.daffodil.lib.schema.annotation.props.Found
import org.apache.daffodil.lib.schema.annotation.props.NotFound
import org.apache.daffodil.lib.schema.annotation.props.SeparatorSuppressionPolicy
import org.apache.daffodil.lib.schema.annotation.props.gen.LengthKind
import org.apache.daffodil.lib.schema.annotation.props.gen.OccursCountKind
import org.apache.daffodil.lib.schema.annotation.props.gen.YesNo
import org.apache.daffodil.lib.schema.annotation.props.gen.*

/**
* Mixin for objects that are shared, but have consistency checks to be run
Expand Down Expand Up @@ -269,6 +267,26 @@ trait Term
.getOrElse(false)
}

final lazy val immediatelyEnclosingParentForEOPElem: Option[Term] = {
Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence May 26, 2026

Choose a reason for hiding this comment

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

Can this be removed? It doesn't look like it's used anywhere, and it is has subtle edge cases where it doesn't necessarily return what one might think it would because it only knows about lexical parents.

val p = optLexicalParent.flatMap {
Comment thread
stevedlawrence marked this conversation as resolved.
case e: ElementBase => Some(e)
case ge: GlobalElementDecl => Some(ge.asRoot)
case s: SequenceTermBase => s.immediatelyEnclosingParentForEOPElem
case c: ChoiceTermBase => Some(c)
case ct: ComplexTypeBase => {
ct.optLexicalParent.flatMap {
case e: ElementBase => Some(e)
case ge: GlobalElementDecl => Some(ge.asRoot)
case _ => {
None
}
}
}
case _ => None
}
p
}

final lazy val immediatelyEnclosingGroupDef: Option[GroupDefLike] = {
optLexicalParent.flatMap { lexicalParent =>
val res: Option[GroupDefLike] = lexicalParent match {
Expand Down Expand Up @@ -562,4 +580,21 @@ trait Term
}
}

final lazy val realElementChildren: Seq[ElementBase] = {
termChildren.flatMap {
case eb: ElementBase => Seq(eb)
case c: Choice => Nil
case mg: ModelGroup => mg.realElementChildren
}
}

lazy val flattenedChildren: IndexedSeq[Term] = termChildren.flatMap { c =>
Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence May 26, 2026

Choose a reason for hiding this comment

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

Does an IndexedSeq provide much benefit with how this is used? I don't think we will usually know what index we an element is, so we'll likely have to do a linear scans regardless. And the way this is used it looks like we do indexOf and then get everything after that index. If this were a normal Seq, I think similar logic could be implemented like this:

val elementPlusNextSiblings = flattendChildren.dropWhile(_ != specificEopChild)
Assert.invariant(!elementPlusNextSibligns.isEmpty)
val nextSiblings = elementPlusNextSiblings.tail
if (nextSiblings.isDefined) {
  // SDE, children exist afterEOPelement
}

We don't really use the indexability, so this might avoid some overheads.

c match {
case eb: ElementBase => IndexedSeq(eb)
case mg: ModelGroup if mg.groupMembers.isEmpty => IndexedSeq(mg)
case s: SequenceTermBase => s.flattenedChildren
case c: ChoiceTermBase => c.flattenedChildren
Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence May 26, 2026

Choose a reason for hiding this comment

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

I'm not sure this is correct for ChoiceTermBase, at least in the context where this is used to detect if there are siblings after this. For example, say we have a schema below, where the first branch of a choice contains an EOP element:

<element name="foo">
  <complexType>
    <choice>
      <element name="bar" dfdl:lengthKind="endOfParent" ... />
      <element name="baz" dfdl:lenghtKind="explicit" ... />
    </choice>
  </complexType>
</element>

In this case, I think the choices flattenedChildren will look like:

IndexedSeq(bar, baz)

And so it will look like baz follows an EOP element and would SDE. But I think this this schema is legal?

Since choiceLengthKind doesn't allow EOP, maybe we just return Nil here, similar to realElementChildren? And maybe choices need to run their checks on individual branches? I'm not sure we can just flatten all branches of a choice into a single list of children.

Or maybe we need a different way to determine if there are things after an EOP element that doesn't require flattening. Maybe checkEndOfParentRestrictions returns a boolean that indicates if it saw an EOP element, and then as we roll up from the recursion we check the result? For xample, maybe something like:

def checkEndOfParentRestrcitions(...) {
  ...

  val sawEOP = term.termChildren.foldLeft(false) { case (child, sawEOP)) =>
    if (sawEOP) {
     // the previous child saw an EOP element, and this child is after it, so error
     SDE(...)
    }
    child.checkEndOfParentRestrictions(...)
  }
  sawEOP
}

This doesn't handle non-represented elements or choice branches, but maybe something along those lines? This is also nice in that it doesn't require allocating any new lists like the flattenedChildren. We're already having to walking the entire tree, so if we can do logic just be return values or something that would be preferred. It's not the standard way we do things, but this parent/sibling logic this reuires kindof makes the normal way difficult.

case _ => Nil
}
}.toIndexedSeq
}
6 changes: 5 additions & 1 deletion 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 @@ -456,7 +456,11 @@ trait AlignedMixin extends GrammarMixin { self: Term =>
}
case LengthKind.Delimited => encodingLengthApprox
case LengthKind.Pattern => encodingLengthApprox
case LengthKind.EndOfParent => LengthMultipleOf(1) // NYI
case LengthKind.EndOfParent =>
// technically the alignment of an EndOfParent element would be the
// alignment of its parent minus our current alignment (i.e alignment of
// prior sibs) but since nothing can follow
Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence May 28, 2026

Choose a reason for hiding this comment

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

This looks like a fragment sentence? Also this comment talks about alignment but this function is about calculating the approximate length. I think LengthMultipleOf(1) is correct, but we might want to update the comment as to why.

LengthMultipleOf(1)
// If an element is lengthKind="prefixed", the element's length is the length
// of the value of the prefix element, which can't be known till runtime
case LengthKind.Prefixed => LengthMultipleOf(1) // NYI (see DAFFODIL-3066)
Expand Down
Loading
Loading