mbeckerle commented on a change in pull request #158: Daffodil 1080 sequences
and separators - preliminary review
URL: https://github.com/apache/incubator-daffodil/pull/158#discussion_r241939408
##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/Term.scala
##########
@@ -434,43 +425,84 @@ trait Term
*/
def isVariableOccurrences: Boolean = false
+ /**
+ * True when a term's immediately enclosing model group is a Sequence.
+ */
+ final lazy val isSequenceChild =
+
immediatelyEnclosingModelGroup.getOrElse(false).isInstanceOf[SequenceTermBase]
+
/**
* The concept of potentially trailing is defined in the DFDL specification.
*
- * It means that the term could have instances that are the last thing in
the sequence group
- * and that are potentially also not present, so the issue of extra
separators being present/absent for
- * instances of the term is relevant.
+ * This concept applies to terms that are direct children of a sequence only.
+ *
+ * It is true for terms that may be absent from the representation, but
furthermore, may be last
+ * in a sequence, so that the notion of whether they are trailing, and so
their separator may not be
+ * present, is a relevant issue.
+ *
+ * If an element is an array, and has some required instances, then it is
not potentially trailing, as some
+ * instances will have to appear, with separators.
+ *
+ * This concept applies only to elements and model groups that have
representation in the data stream.
+ *
+ * Previously there was a misguided notion that since only DFDL elements can
have minOccurs/maxOccurs
+ * that this notion of potentially trailing didn't apply to model groups.
(Sequences and Choices, the other
+ * kind of Term). But this is not the case.
+ *
+ * A sequence/choice which has no framing, and whose content doesn't exist -
no child elements, any contained
+ * model groups recursively with no framing and no content - such a model
group effectively "dissapears" from
+ * the data stream, and in some cases need not have a separator.
*
- * This currently can only be true for elements, however, it is defined for
terms because
- * the DFDL spec defines
- * the concept of a potentially trailing group as well. Though the current
draft of the DFDL spec
- * doesn't USE this concept of potentially trailing group, that is likely to
be corrected at some
- * point.
+ * This is computed by way of couldBePotentiallyTrailing. This value means
that the term, in isolation, looking only
+ * at its own characteristics, disregarding its following siblings in any
given sequence, has the characteristics
+ * of being potentially trailing.
+ *
+ * Then that is combined with information about following siblings in a
sequence to determine if a given term, that
+ * is a child of a sequence, is in fact potentially trailing within that
sequence.
+ *
+ * These two concepts are mutually recursive, since a sequence that is
entirely composed of potentially trailing children
+ * satisfies couldBePotentialyTrailing in whatever sequence encloses it.
*/
- final lazy val isPotentiallyTrailing = {
- if (!isRepresented) false
- else if (!isRequiredOrComputed ||
- (isRequiredOrComputed && isLastDeclaredRepresentedInSequence &&
isVariableOccurrences)) {
- val es = nearestEnclosingSequence
- val res = es match {
- case None => true
- case Some(s) => {
- val allRequired = s.groupMembers.filter(_.isRequiredOrComputed)
- if (allRequired.isEmpty) true
- else {
- val lastDeclaredRequired = allRequired.last
- val thisLoc = s.groupMembers.indexOf(this)
- val lastDeclaredRequiredLoc =
s.groupMembers.indexOf(lastDeclaredRequired)
- val res = lastDeclaredRequiredLoc <= thisLoc
- res
- }
+ final lazy val isPotentiallyTrailing: Boolean = LV('isPotentiallyTrailing) {
+ Assert.usage(isSequenceChild || parent.isInstanceOf[ComplexTypeBase])
+ val res =
+ couldBePotentiallyTrailing &&
+ this.laterSiblings.forall { _.isPotentiallyTrailing }
+ res
+ }.value
+
+ // (isRequiredInInfoset && isLastDeclaredRepresentedInSequence &&
isVariableOccurrences)) {
Review comment:
Remove commented code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
