stevedlawrence commented on a change in pull request #214: Sequences and
Separators Refactoring and Rewrite
URL: https://github.com/apache/incubator-daffodil/pull/214#discussion_r285194335
##########
File path:
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/processors/unparsers/SeparatedSequenceUnparsers.scala
##########
@@ -22,238 +22,453 @@ import org.apache.daffodil.processors.{
ElementRuntimeData, SequenceRuntimeData,
import org.apache.daffodil.schema.annotation.props.SeparatorSuppressionPolicy
import org.apache.daffodil.schema.annotation.props.gen.{ SeparatorPosition }
import org.apache.daffodil.infoset.DIElement
+import org.apache.daffodil.processors.ModelGroupRuntimeData
+import org.apache.daffodil.util.Maybe
+import scala.collection.mutable.Buffer
+import SeparatorSuppressionPolicy._
+import SeparatorPosition._
/**
* DFDL Spec. section 14.2.3 specifies only a few different behaviors
* for separator suppression. Each has an algorithm.
*/
-sealed trait SeparatorSuppressionMode extends Serializable {
-
- /**
- * Determines if we should suppress a zero length element and
- * separator.
- *
- * Checks if the length is zero if necessary to decide.
- *
- * Returns Suppress or IfTrailing when the element is, in fact, zero length,
- * and the algorithm wants those to be suppressed.
- * Returns DoNotSuppress otherwise.
- */
- def shouldSuppressIfZeroLength(state: UState, infosetElement: DIElement):
SeparatorSuppressionAction
-
- def shouldDoExtraSeparators = false
-
- def checkArrayPosAgainstMaxOccurs(state: UState, maxRepeats: Long): Boolean
= true
-
-}
-
-object SeparatorSuppressionMode {
- type Type = SeparatorSuppressionMode
- import SeparatorSuppressionAction._
-
- object None extends Type {
- def shouldSuppressIfZeroLength(state: UState, infosetElement: DIElement) =
Assert.usageError("not to be called.")
- }
- object FixedOrExpression extends Type {
- def shouldSuppressIfZeroLength(state: UState, infosetElement: DIElement) =
DoNotSuppress
- }
-
- class SuppressAnyEmpty(zeroLengthDetector: ZeroLengthDetector) extends Type {
- def shouldSuppressIfZeroLength(state: UState, infosetElement: DIElement) =
{
- val isZL = zeroLengthDetector.isZeroLength(infosetElement)
- if (isZL) Suppress
- else DoNotSuppress
- }
- }
-
- trait CheckArrayPosMixin { self: SeparatorSuppressionMode =>
- override def checkArrayPosAgainstMaxOccurs(state: UState, maxRepeats:
Long): Boolean = {
- state.arrayPos <= maxRepeats
- }
- }
-
- class ImplicitSuppressAnyEmpty(zeroLengthDetector: ZeroLengthDetector)
- extends SuppressAnyEmpty(zeroLengthDetector)
- with CheckArrayPosMixin
-
- object ImplicitNeverOrNotPotentiallyTrailing extends Type
- with CheckArrayPosMixin {
- def shouldSuppressIfZeroLength(state: UState, infosetElement: DIElement) =
DoNotSuppress
-
- override def shouldDoExtraSeparators = true
-
- }
-
- class ImplicitPotentiallyTrailing(zeroLengthDetector: ZeroLengthDetector)
extends Type
- with CheckArrayPosMixin {
- def shouldSuppressIfZeroLength(state: UState, infosetElement: DIElement) =
{
- val isZL = zeroLengthDetector.isZeroLength(infosetElement)
- if (isZL) IfTrailing
- else DoNotSuppress
- }
- }
-}
-
-sealed trait SeparatorSuppressionAction extends Serializable
-object SeparatorSuppressionAction {
- type Type = SeparatorSuppressionAction
- case object Suppress extends Type
- case object DoNotSuppress extends Type
- case object IfTrailing extends Type
-}
+//sealed trait SeparatorSuppressionMode extends Serializable
+//
+///**
+// * Different separator suppression modes correspond to the different
behaviors
+// * of the Section 14.2.2 prose and Table 47.
+// */
+//object SeparatorSuppressionMode {
+// type Type = SeparatorSuppressionMode
+//
+// object None extends Type
+//
+// object FixedOrExpression extends Type
+//
+// object SuppressAnyEmpty extends Type
+//
+// object ImplicitSuppressAnyEmpty extends Type
+//
+// object ImplicitNeverOrNotPotentiallyTrailing extends Type
+//
+// object ImplicitPotentiallyTrailing extends Type
+//}
trait Separated { self: SequenceChildUnparser =>
def sep: Unparser
def spos: SeparatorPosition
def ssp: SeparatorSuppressionPolicy
- def ssAlgorithm: SeparatorSuppressionMode
+ def zeroLengthDetector: ZeroLengthDetector
+ def isPotentiallyTrailing: Boolean
+
+ def isKnownStaticallyNotToSuppressSeparator: Boolean
+
+ def isDeclaredLast: Boolean
+
+ def isPositional: Boolean
val childProcessors = Vector(childUnparser, sep)
}
-class ScalarOrderedSeparatedSequenceChildUnparser(
- childUnparser: Unparser,
+sealed abstract class
ScalarOrderedSeparatedSequenceChildUnparserBase(childUnparser: Unparser,
srd: SequenceRuntimeData,
trd: TermRuntimeData,
- val sep: Unparser,
- val spos: SeparatorPosition,
- val ssp: SeparatorSuppressionPolicy,
- val ssAlgorithm: SeparatorSuppressionMode)
+ override val sep: Unparser,
+ override val spos: SeparatorPosition,
+ override val ssp: SeparatorSuppressionPolicy,
+ override val zeroLengthDetector: ZeroLengthDetector,
+ override val isPotentiallyTrailing: Boolean,
+ override val isKnownStaticallyNotToSuppressSeparator: Boolean,
+ override val isPositional: Boolean,
+ override val isDeclaredLast: Boolean)
extends SequenceChildUnparser(childUnparser, srd, trd)
with Separated {
override def unparse(state: UState) = childUnparser.unparse1(state)
}
-class RepOrderedSeparatedSequenceChildUnparser(
- childUnparser: Unparser,
+class ScalarOrderedSeparatedSequenceChildUnparser(childUnparser: Unparser,
+ srd: SequenceRuntimeData,
+ trd: TermRuntimeData,
+ sep: Unparser,
+ spos: SeparatorPosition,
+ ssp: SeparatorSuppressionPolicy,
+ zlDetector: ZeroLengthDetector,
+ isPotentiallyTrailing: Boolean,
+ isKnownStaticallyNotToSuppressSeparator: Boolean,
+ isPositional: Boolean,
+ isDeclaredLast: Boolean)
+ extends ScalarOrderedSeparatedSequenceChildUnparserBase(childUnparser, srd,
trd, sep, spos, ssp,
+ zlDetector, isPotentiallyTrailing,
isKnownStaticallyNotToSuppressSeparator, isPositional,
+ isDeclaredLast)
+
+class RepOrderedSeparatedSequenceChildUnparser(childUnparser: Unparser,
srd: SequenceRuntimeData,
erd: ElementRuntimeData,
- val sep: Unparser,
- val spos: SeparatorPosition,
- val ssp: SeparatorSuppressionPolicy, // need for diagnostics perhaps
- val ssAlgorithm: SeparatorSuppressionMode)
+ override val sep: Unparser,
+ override val spos: SeparatorPosition,
+ override val ssp: SeparatorSuppressionPolicy, // need for diagnostics perhaps
+ override val zeroLengthDetector: ZeroLengthDetector,
+ override val isPotentiallyTrailing: Boolean,
+ override val isKnownStaticallyNotToSuppressSeparator: Boolean,
+ override val isPositional: Boolean,
+ override val isDeclaredLast: Boolean)
extends RepeatingChildUnparser(childUnparser, srd, erd)
with Separated {
- override def checkArrayPosAgainstMaxOccurs(state: UState) =
ssAlgorithm.checkArrayPosAgainstMaxOccurs(state, maxRepeats(state))
+ override def checkArrayPosAgainstMaxOccurs(state: UState) =
+ state.arrayPos <= maxRepeats(state)
}
-class OrderedSeparatedSequenceUnparser(
- rd: SequenceRuntimeData,
+class OrderedSeparatedSequenceUnparser(rd: SequenceRuntimeData,
ssp: SeparatorSuppressionPolicy,
spos: SeparatorPosition,
sep: Unparser,
childUnparsersArg: Vector[SequenceChildUnparser])
- extends OrderedSequenceUnparserBase(rd, childUnparsersArg) {
-
- private val childUnparsers =
childUnparsersArg.asInstanceOf[Seq[SequenceChildUnparser with Separated]]
-
- /**
- * True if requires special treatment in the unparse processing loop, as
occurrences
- * of later sequence children can influence whether possible trailing
separators
- * from earlier sequence children are actually trailing or not.
- */
- private type IPT = SeparatorSuppressionMode.ImplicitPotentiallyTrailing
+ extends OrderedSequenceUnparserBase(rd, childUnparsersArg :+ sep) {
- private val hasTrailingSeparatorSuppression = {
- childUnparsers.last.ssAlgorithm.isInstanceOf[IPT]
- }
+ private val childUnparsers =
+ childUnparsersArg.asInstanceOf[Seq[SequenceChildUnparser with Separated]]
/**
- * Unparses one occurrence.
+ * Unparses one occurrence with associated separator (non-suppressable).
*/
- protected def unparseOne(
- unparser: SequenceChildUnparser,
+ protected def unparseOne(unparser: SequenceChildUnparser,
trd: TermRuntimeData,
state: UState): Unit = {
if (trd.isRepresented) {
- if ((spos eq SeparatorPosition.Prefix)) {
- sep.unparse1(state)
- } else if ((spos eq SeparatorPosition.Infix) && state.groupPos > 1) {
- sep.unparse1(state)
+ spos match {
+ case Prefix => {
+ sep.unparse1(state)
+ unparser.unparse1(state)
+ }
+ case Infix if (state.groupPos > 1) => {
+ sep.unparse1(state)
+ unparser.unparse1(state)
+ }
+ case Infix => {
+ unparser.unparse1(state)
+ }
+ case Postfix => {
+ unparser.unparse1(state)
+ sep.unparse1(state)
+ }
}
+ } else {
+ Assert.invariant(!trd.isRepresented)
+ unparser.unparse1(state)
}
-
- unparser.unparse1(state)
-
- if ((spos eq SeparatorPosition.Postfix) && trd.isRepresented) {
- sep.unparse1(state)
- }
- }
-
- /**
- * Unparses a zero-length occurrence, without the separator. This is so that
- * any statements or other side-effects (discriminators, setVariable, etc.)
- * will occur.
- */
- private def unparseZeroLengthWithoutSeparatorForSideEffect(
- unparser: SequenceChildUnparser,
- trd: TermRuntimeData,
- state: UState): Unit = {
- //
- // Unfortunately there's no way to confirm that this produced zero length
- // because of the possible buffering going on in the unparser.
- // We'd have to depend on intricate details of the unparser behavior to do
this,
- // and that's unwise from separation-of-concerns perspective.
- //
- unparser.unparse1(state)
}
/**
* Unparses the separator only.
*
- * Does not deals with infix boundary condition, because
- * the counting of the potential trailing separators takes
- * this into account.
+ * Does not deals with infix boundary condition.
*/
private def unparseJustSeparator(state: UState): Unit = {
sep.unparse1(state)
}
/**
- * Returns 1, or 0 if infix separator, and this is the first thing
- * in the sequence meaning there is no separator for it.
+ * Unparses the separator only.
*
- * However, if we're not doing trailing separator suppression, always
- * returns 0.
+ * Does not have to deal with infix and first child.
*/
- private def suppressedTrailingSeparatorIncrement(unparser:
SequenceChildUnparser with Separated, state: UState): Int = {
- Assert.usage(unparser.trd.isRepresented)
- val notIPT = !unparser.ssAlgorithm.isInstanceOf[IPT]
- val result =
- if (notIPT)
- 0
- else {
- val infixAndFirst = (spos eq SeparatorPosition.Infix) &&
state.groupPos == 1
- if (infixAndFirst)
- 0
- else
- 1
- }
- result
+ private def unparseJustSeparatorWithTrailingSuppression(trd: TermRuntimeData,
+ state: UState,
+ trailingSuspendedOps:
Buffer[SuppressableSeparatorUnparserSuspendableOperation]): Unit = {
+ // TODO: merge these two objects. We can allocate just one thing here.
+ // lazy because many sequences contain just one thing with infix position,
+ // so those degenerate cases won't allocate these objects.
Review comment:
Not sure I understand the "lazy" comment. Both suspendableOp and
supppressableSep are always going to be used below. Maybe they used to be class
members instead of allocated in the function? I *think* they can be made
non-lazy now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
