stevedlawrence commented on code in PR #1338:
URL: https://github.com/apache/daffodil/pull/1338#discussion_r1941064848
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala:
##########
@@ -1322,60 +1328,75 @@ trait ElementBaseGrammarMixin
* as well, by not enclosing the body in a specified length enforcer.
*/
private def specifiedLength(bodyArg: => Gram) = {
- lazy val body = bodyArg
- lazy val bitsMultiplier = lengthUnits match {
- case LengthUnits.Bits => 1
- case LengthUnits.Bytes => 8
- case LengthUnits.Characters if knownEncodingIsFixedWidth =>
this.knownEncodingWidthInBits
- case _ => 0 // zero means can't multiply to get width in bits.
- }
- val lk = lengthKind
- lk match {
- case LengthKind.Delimited => body
- case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
- case LengthKind.Explicit if bitsMultiplier != 0 =>
- new SpecifiedLengthExplicit(this, body, bitsMultiplier)
- case LengthKind.Explicit => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthExplicitCharacters(this, body)
- }
- case LengthKind.Prefixed if (bitsMultiplier != 0) =>
- new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
- case LengthKind.Prefixed => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthPrefixedCharacters(this, body)
- }
- case LengthKind.Implicit
- if isSimpleType && primType == PrimType.String &&
- encodingInfo.knownEncodingIsFixedWidth => {
- //
- // Important case to optimize
- // If we can convert to a number of bits, then we should do so
- //
- val nBits =
encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
- new SpecifiedLengthImplicit(this, body, nBits)
+ // we need this to evaluate before we wrap in specified length parser,
+ // so it can do any internal checks for example blobValue's check for
+ // non-explicit lengthKind
+ val body = bodyArg
+ if (
+ isSimpleType && impliedRepresentation == Representation.Text ||
+ isSimpleType && isNillable ||
+ isComplexType ||
+ lengthKind == LengthKind.Prefixed ||
+ isSimpleType && primType == PrimType.HexBinary && lengthKind ==
LengthKind.Pattern
+ ) {
Review Comment:
It might be worth setting this condition to a val to make it more self
documenting. Even with that, it might also be worth adding comment clarifying
why we do this. Maybe something like"
```scala
// there are essentially two categories of processors that read/write data
input/output
// stream: those that calculate lengths themselves and those that expect
another
// processor to calculate the length and set the bit limit which this
processor will use as
// the length. The following determines if this element requires another
processor to
// calculate and set the bit limit, and if so adds the appropriate grammar
to do that
val bodyRequiresSpecifiedLengthBitLimit = isSimpleType &&
impliedRepresentation == Representation.Text || ...
if (!bodyRequiresSpecifiedLengthBitLimit) body
else lengthKind match {
// all the cases that add various specified length grammars that body
depends on
}
```
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala:
##########
@@ -1322,60 +1328,75 @@ trait ElementBaseGrammarMixin
* as well, by not enclosing the body in a specified length enforcer.
*/
private def specifiedLength(bodyArg: => Gram) = {
- lazy val body = bodyArg
- lazy val bitsMultiplier = lengthUnits match {
- case LengthUnits.Bits => 1
- case LengthUnits.Bytes => 8
- case LengthUnits.Characters if knownEncodingIsFixedWidth =>
this.knownEncodingWidthInBits
- case _ => 0 // zero means can't multiply to get width in bits.
- }
- val lk = lengthKind
- lk match {
- case LengthKind.Delimited => body
- case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
- case LengthKind.Explicit if bitsMultiplier != 0 =>
- new SpecifiedLengthExplicit(this, body, bitsMultiplier)
- case LengthKind.Explicit => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthExplicitCharacters(this, body)
- }
- case LengthKind.Prefixed if (bitsMultiplier != 0) =>
- new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
- case LengthKind.Prefixed => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthPrefixedCharacters(this, body)
- }
- case LengthKind.Implicit
- if isSimpleType && primType == PrimType.String &&
- encodingInfo.knownEncodingIsFixedWidth => {
- //
- // Important case to optimize
- // If we can convert to a number of bits, then we should do so
- //
- val nBits =
encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
- new SpecifiedLengthImplicit(this, body, nBits)
+ // we need this to evaluate before we wrap in specified length parser,
+ // so it can do any internal checks for example blobValue's check for
+ // non-explicit lengthKind
+ val body = bodyArg
+ if (
+ isSimpleType && impliedRepresentation == Representation.Text ||
+ isSimpleType && isNillable ||
+ isComplexType ||
+ lengthKind == LengthKind.Prefixed ||
+ isSimpleType && primType == PrimType.HexBinary && lengthKind ==
LengthKind.Pattern
+ ) {
+ lazy val bitsMultiplier = lengthUnits match {
+ case LengthUnits.Bits => 1
+ case LengthUnits.Bytes => 8
+ case LengthUnits.Characters if knownEncodingIsFixedWidth =>
+ this.knownEncodingWidthInBits
+ case _ => 0 // zero means can't multiply to get width in bits.
}
- case LengthKind.Implicit if isSimpleType && primType == PrimType.String
=>
- new SpecifiedLengthImplicitCharacters(this, body,
this.maxLength.longValue)
-
- case LengthKind.Implicit if isSimpleType && primType ==
PrimType.HexBinary =>
- new SpecifiedLengthImplicit(this, body, this.maxLength.longValue *
bitsMultiplier)
- case LengthKind.Implicit
- if isSimpleType && impliedRepresentation == Representation.Binary =>
- new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
- case LengthKind.Implicit if isComplexType =>
- body // for complex types, implicit means "roll up from the bottom"
- case LengthKind.EndOfParent if isComplexType =>
- notYetImplemented("lengthKind='endOfParent' for complex type")
- case LengthKind.EndOfParent =>
- notYetImplemented("lengthKind='endOfParent' for simple type")
- case _ => {
- // TODO: implement other specified length like end of parent
- // for now, no restriction
- body
+ val lk = lengthKind
+ lk match {
+ case LengthKind.Delimited => body
Review Comment:
lengthKind=Delimted just uses body--thoughts on adding a `|| lengthKind ==
LengthKind.Delimited` to the condition above, making it more clear that all
delimited processors do the length calculations themselves and don't need a
specified length processor?
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala:
##########
@@ -1322,60 +1328,75 @@ trait ElementBaseGrammarMixin
* as well, by not enclosing the body in a specified length enforcer.
*/
private def specifiedLength(bodyArg: => Gram) = {
- lazy val body = bodyArg
- lazy val bitsMultiplier = lengthUnits match {
- case LengthUnits.Bits => 1
- case LengthUnits.Bytes => 8
- case LengthUnits.Characters if knownEncodingIsFixedWidth =>
this.knownEncodingWidthInBits
- case _ => 0 // zero means can't multiply to get width in bits.
- }
- val lk = lengthKind
- lk match {
- case LengthKind.Delimited => body
- case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
- case LengthKind.Explicit if bitsMultiplier != 0 =>
- new SpecifiedLengthExplicit(this, body, bitsMultiplier)
- case LengthKind.Explicit => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthExplicitCharacters(this, body)
- }
- case LengthKind.Prefixed if (bitsMultiplier != 0) =>
- new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
- case LengthKind.Prefixed => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthPrefixedCharacters(this, body)
- }
- case LengthKind.Implicit
- if isSimpleType && primType == PrimType.String &&
- encodingInfo.knownEncodingIsFixedWidth => {
- //
- // Important case to optimize
- // If we can convert to a number of bits, then we should do so
- //
- val nBits =
encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
- new SpecifiedLengthImplicit(this, body, nBits)
+ // we need this to evaluate before we wrap in specified length parser,
+ // so it can do any internal checks for example blobValue's check for
+ // non-explicit lengthKind
+ val body = bodyArg
+ if (
+ isSimpleType && impliedRepresentation == Representation.Text ||
+ isSimpleType && isNillable ||
+ isComplexType ||
+ lengthKind == LengthKind.Prefixed ||
+ isSimpleType && primType == PrimType.HexBinary && lengthKind ==
LengthKind.Pattern
+ ) {
+ lazy val bitsMultiplier = lengthUnits match {
+ case LengthUnits.Bits => 1
+ case LengthUnits.Bytes => 8
+ case LengthUnits.Characters if knownEncodingIsFixedWidth =>
+ this.knownEncodingWidthInBits
+ case _ => 0 // zero means can't multiply to get width in bits.
}
- case LengthKind.Implicit if isSimpleType && primType == PrimType.String
=>
- new SpecifiedLengthImplicitCharacters(this, body,
this.maxLength.longValue)
-
- case LengthKind.Implicit if isSimpleType && primType ==
PrimType.HexBinary =>
- new SpecifiedLengthImplicit(this, body, this.maxLength.longValue *
bitsMultiplier)
- case LengthKind.Implicit
- if isSimpleType && impliedRepresentation == Representation.Binary =>
- new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
- case LengthKind.Implicit if isComplexType =>
- body // for complex types, implicit means "roll up from the bottom"
- case LengthKind.EndOfParent if isComplexType =>
- notYetImplemented("lengthKind='endOfParent' for complex type")
- case LengthKind.EndOfParent =>
- notYetImplemented("lengthKind='endOfParent' for simple type")
- case _ => {
- // TODO: implement other specified length like end of parent
- // for now, no restriction
- body
+ val lk = lengthKind
+ lk match {
+ case LengthKind.Delimited => body
+ case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
+ case LengthKind.Explicit if bitsMultiplier != 0 =>
+ new SpecifiedLengthExplicit(this, body, bitsMultiplier)
+ case LengthKind.Explicit => {
+ Assert.invariant(!knownEncodingIsFixedWidth)
+ Assert.invariant(lengthUnits eq LengthUnits.Characters)
+ new SpecifiedLengthExplicitCharacters(this, body)
+ }
+ case LengthKind.Prefixed if (bitsMultiplier != 0) =>
+ new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
+ case LengthKind.Prefixed => {
+ Assert.invariant(!knownEncodingIsFixedWidth)
+ Assert.invariant(lengthUnits eq LengthUnits.Characters)
+ new SpecifiedLengthPrefixedCharacters(this, body)
+ }
+ case LengthKind.Implicit
+ if isSimpleType && primType == PrimType.String &&
+ encodingInfo.knownEncodingIsFixedWidth => {
+ //
+ // Important case to optimize
+ // If we can convert to a number of bits, then we should do so
+ //
+ val nBits =
+
encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
+ new SpecifiedLengthImplicit(this, body, nBits)
+ }
+ case LengthKind.Implicit if isSimpleType && primType ==
PrimType.String =>
+ new SpecifiedLengthImplicitCharacters(this, body,
this.maxLength.longValue)
+
+ case LengthKind.Implicit if isSimpleType && primType ==
PrimType.HexBinary =>
+ new SpecifiedLengthImplicit(this, body, this.maxLength.longValue *
bitsMultiplier)
+ case LengthKind.Implicit
+ if isSimpleType && impliedRepresentation == Representation.Binary
=>
+ new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
+ case LengthKind.Implicit if isComplexType =>
+ body // for complex types, implicit means "roll up from the bottom"
+ case LengthKind.EndOfParent if isComplexType =>
+ notYetImplemented("lengthKind='endOfParent' for complex type")
+ case LengthKind.EndOfParent =>
+ notYetImplemented("lengthKind='endOfParent' for simple type")
+ case _ => {
Review Comment:
This default case isn't covered, which is probably a good thing--it probably
means the condition at the top is catching all cases that really don't need a
specified length parser and we aren't just accidentally falling into this case.
I would suggest we either remove this case, or if Scala complains about a
non-exhaustive match change it an an `Assert.impossible` with codecov disabled.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala:
##########
@@ -1322,60 +1328,75 @@ trait ElementBaseGrammarMixin
* as well, by not enclosing the body in a specified length enforcer.
*/
private def specifiedLength(bodyArg: => Gram) = {
- lazy val body = bodyArg
- lazy val bitsMultiplier = lengthUnits match {
- case LengthUnits.Bits => 1
- case LengthUnits.Bytes => 8
- case LengthUnits.Characters if knownEncodingIsFixedWidth =>
this.knownEncodingWidthInBits
- case _ => 0 // zero means can't multiply to get width in bits.
- }
- val lk = lengthKind
- lk match {
- case LengthKind.Delimited => body
- case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
- case LengthKind.Explicit if bitsMultiplier != 0 =>
- new SpecifiedLengthExplicit(this, body, bitsMultiplier)
- case LengthKind.Explicit => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthExplicitCharacters(this, body)
- }
- case LengthKind.Prefixed if (bitsMultiplier != 0) =>
- new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
- case LengthKind.Prefixed => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthPrefixedCharacters(this, body)
- }
- case LengthKind.Implicit
- if isSimpleType && primType == PrimType.String &&
- encodingInfo.knownEncodingIsFixedWidth => {
- //
- // Important case to optimize
- // If we can convert to a number of bits, then we should do so
- //
- val nBits =
encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
- new SpecifiedLengthImplicit(this, body, nBits)
+ // we need this to evaluate before we wrap in specified length parser,
+ // so it can do any internal checks for example blobValue's check for
+ // non-explicit lengthKind
+ val body = bodyArg
+ if (
+ isSimpleType && impliedRepresentation == Representation.Text ||
+ isSimpleType && isNillable ||
+ isComplexType ||
+ lengthKind == LengthKind.Prefixed ||
+ isSimpleType && primType == PrimType.HexBinary && lengthKind ==
LengthKind.Pattern
+ ) {
+ lazy val bitsMultiplier = lengthUnits match {
+ case LengthUnits.Bits => 1
+ case LengthUnits.Bytes => 8
+ case LengthUnits.Characters if knownEncodingIsFixedWidth =>
+ this.knownEncodingWidthInBits
+ case _ => 0 // zero means can't multiply to get width in bits.
}
- case LengthKind.Implicit if isSimpleType && primType == PrimType.String
=>
- new SpecifiedLengthImplicitCharacters(this, body,
this.maxLength.longValue)
-
- case LengthKind.Implicit if isSimpleType && primType ==
PrimType.HexBinary =>
- new SpecifiedLengthImplicit(this, body, this.maxLength.longValue *
bitsMultiplier)
- case LengthKind.Implicit
- if isSimpleType && impliedRepresentation == Representation.Binary =>
- new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
- case LengthKind.Implicit if isComplexType =>
- body // for complex types, implicit means "roll up from the bottom"
- case LengthKind.EndOfParent if isComplexType =>
- notYetImplemented("lengthKind='endOfParent' for complex type")
- case LengthKind.EndOfParent =>
- notYetImplemented("lengthKind='endOfParent' for simple type")
- case _ => {
- // TODO: implement other specified length like end of parent
- // for now, no restriction
- body
+ val lk = lengthKind
+ lk match {
+ case LengthKind.Delimited => body
+ case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
+ case LengthKind.Explicit if bitsMultiplier != 0 =>
+ new SpecifiedLengthExplicit(this, body, bitsMultiplier)
+ case LengthKind.Explicit => {
+ Assert.invariant(!knownEncodingIsFixedWidth)
+ Assert.invariant(lengthUnits eq LengthUnits.Characters)
+ new SpecifiedLengthExplicitCharacters(this, body)
+ }
+ case LengthKind.Prefixed if (bitsMultiplier != 0) =>
+ new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
+ case LengthKind.Prefixed => {
+ Assert.invariant(!knownEncodingIsFixedWidth)
+ Assert.invariant(lengthUnits eq LengthUnits.Characters)
+ new SpecifiedLengthPrefixedCharacters(this, body)
+ }
+ case LengthKind.Implicit
+ if isSimpleType && primType == PrimType.String &&
+ encodingInfo.knownEncodingIsFixedWidth => {
+ //
+ // Important case to optimize
+ // If we can convert to a number of bits, then we should do so
+ //
+ val nBits =
+
encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
+ new SpecifiedLengthImplicit(this, body, nBits)
+ }
+ case LengthKind.Implicit if isSimpleType && primType ==
PrimType.String =>
+ new SpecifiedLengthImplicitCharacters(this, body,
this.maxLength.longValue)
+
+ case LengthKind.Implicit if isSimpleType && primType ==
PrimType.HexBinary =>
+ new SpecifiedLengthImplicit(this, body, this.maxLength.longValue *
bitsMultiplier)
+ case LengthKind.Implicit
+ if isSimpleType && impliedRepresentation == Representation.Binary
=>
+ new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
+ case LengthKind.Implicit if isComplexType =>
+ body // for complex types, implicit means "roll up from the bottom"
Review Comment:
Should we change the condition at the top to `isComplex && lengthKind !=
lengthKind.Implicit` and remove this case? I'm just wondering if there's value
in making it so non of these cases have `body` as the result if that makes
things more clear exactly when we need a specified length parser?
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesLengthKind.scala:
##########
@@ -186,21 +185,11 @@ case class HexBinaryEndOfBitLimit(e: ElementBase) extends
Terminal(e, true) {
case class HexBinaryLengthPrefixed(e: ElementBase) extends Terminal(e, true) {
Review Comment:
I think this comment still applies to the new changes? I think it's fine to
keep the `lazy val hexBinaryLengthPrefixed` grammar, but we can probably remove
class and replace its uses with the `HexBinaryEndOfBitLimit` class.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/processors/parsers/BinaryNumberTraits.scala:
##########
@@ -165,3 +165,21 @@ trait PrefixedLengthParserMixin {
}
}
}
+
+/**
+ * This mixin doesn't require parsing the prefix length element and just uses
+ * the state's bitLimit and position to get the bitLength instead
Review Comment:
This comment is a little confusing out of context, since it isn't really
clear why prefix length elements are mentioned since this doesn't really seem
to be prefixed length related. This could also in theory be used on many of the
parsers that rely on bit limit for length, which isn't specific to prefixed
length. Maybe instead say something like
> Some parsers do not calculate their own length, but instead expect another
parser to set the bit limit and then they use that bit limit as the length. An
example of this is prefix length parsers. This trait can be used by those
parsers to do determine the length based on the bitLimit and position.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala:
##########
@@ -1322,60 +1328,75 @@ trait ElementBaseGrammarMixin
* as well, by not enclosing the body in a specified length enforcer.
*/
private def specifiedLength(bodyArg: => Gram) = {
- lazy val body = bodyArg
- lazy val bitsMultiplier = lengthUnits match {
- case LengthUnits.Bits => 1
- case LengthUnits.Bytes => 8
- case LengthUnits.Characters if knownEncodingIsFixedWidth =>
this.knownEncodingWidthInBits
- case _ => 0 // zero means can't multiply to get width in bits.
- }
- val lk = lengthKind
- lk match {
- case LengthKind.Delimited => body
- case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
- case LengthKind.Explicit if bitsMultiplier != 0 =>
- new SpecifiedLengthExplicit(this, body, bitsMultiplier)
- case LengthKind.Explicit => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthExplicitCharacters(this, body)
- }
- case LengthKind.Prefixed if (bitsMultiplier != 0) =>
- new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
- case LengthKind.Prefixed => {
- Assert.invariant(!knownEncodingIsFixedWidth)
- Assert.invariant(lengthUnits eq LengthUnits.Characters)
- new SpecifiedLengthPrefixedCharacters(this, body)
- }
- case LengthKind.Implicit
- if isSimpleType && primType == PrimType.String &&
- encodingInfo.knownEncodingIsFixedWidth => {
- //
- // Important case to optimize
- // If we can convert to a number of bits, then we should do so
- //
- val nBits =
encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
- new SpecifiedLengthImplicit(this, body, nBits)
+ // we need this to evaluate before we wrap in specified length parser,
+ // so it can do any internal checks for example blobValue's check for
+ // non-explicit lengthKind
+ val body = bodyArg
+ if (
+ isSimpleType && impliedRepresentation == Representation.Text ||
+ isSimpleType && isNillable ||
+ isComplexType ||
+ lengthKind == LengthKind.Prefixed ||
+ isSimpleType && primType == PrimType.HexBinary && lengthKind ==
LengthKind.Pattern
+ ) {
+ lazy val bitsMultiplier = lengthUnits match {
+ case LengthUnits.Bits => 1
+ case LengthUnits.Bytes => 8
+ case LengthUnits.Characters if knownEncodingIsFixedWidth =>
+ this.knownEncodingWidthInBits
+ case _ => 0 // zero means can't multiply to get width in bits.
}
- case LengthKind.Implicit if isSimpleType && primType == PrimType.String
=>
- new SpecifiedLengthImplicitCharacters(this, body,
this.maxLength.longValue)
-
- case LengthKind.Implicit if isSimpleType && primType ==
PrimType.HexBinary =>
- new SpecifiedLengthImplicit(this, body, this.maxLength.longValue *
bitsMultiplier)
- case LengthKind.Implicit
- if isSimpleType && impliedRepresentation == Representation.Binary =>
- new SpecifiedLengthImplicit(this, body, implicitBinaryLengthInBits)
- case LengthKind.Implicit if isComplexType =>
- body // for complex types, implicit means "roll up from the bottom"
- case LengthKind.EndOfParent if isComplexType =>
- notYetImplemented("lengthKind='endOfParent' for complex type")
- case LengthKind.EndOfParent =>
- notYetImplemented("lengthKind='endOfParent' for simple type")
- case _ => {
- // TODO: implement other specified length like end of parent
- // for now, no restriction
- body
+ val lk = lengthKind
+ lk match {
+ case LengthKind.Delimited => body
+ case LengthKind.Pattern => new SpecifiedLengthPattern(this, body)
+ case LengthKind.Explicit if bitsMultiplier != 0 =>
+ new SpecifiedLengthExplicit(this, body, bitsMultiplier)
+ case LengthKind.Explicit => {
+ Assert.invariant(!knownEncodingIsFixedWidth)
+ Assert.invariant(lengthUnits eq LengthUnits.Characters)
+ new SpecifiedLengthExplicitCharacters(this, body)
+ }
+ case LengthKind.Prefixed if (bitsMultiplier != 0) =>
+ new SpecifiedLengthPrefixed(this, body, bitsMultiplier)
+ case LengthKind.Prefixed => {
+ Assert.invariant(!knownEncodingIsFixedWidth)
+ Assert.invariant(lengthUnits eq LengthUnits.Characters)
+ new SpecifiedLengthPrefixedCharacters(this, body)
+ }
+ case LengthKind.Implicit
+ if isSimpleType && primType == PrimType.String &&
+ encodingInfo.knownEncodingIsFixedWidth => {
+ //
+ // Important case to optimize
+ // If we can convert to a number of bits, then we should do so
+ //
+ val nBits =
+
encodingInfo.knownFixedWidthEncodingInCharsToBits(this.maxLength.longValue)
+ new SpecifiedLengthImplicit(this, body, nBits)
+ }
+ case LengthKind.Implicit if isSimpleType && primType ==
PrimType.String =>
+ new SpecifiedLengthImplicitCharacters(this, body,
this.maxLength.longValue)
+
+ case LengthKind.Implicit if isSimpleType && primType ==
PrimType.HexBinary =>
+ new SpecifiedLengthImplicit(this, body, this.maxLength.longValue *
bitsMultiplier)
Review Comment:
This case has no coverage, which I think makes sense since only prefixed
length hexBinary uses a specified length parser. All other hexBinary parsers
calculate their own lengths. Should we remove this case to avoid dead code and
possible confusion with how hexBinary is implemented?
##########
daffodil-runtime1-unparser/src/main/scala/org/apache/daffodil/unparsers/runtime1/SpecifiedLengthUnparsers.scala:
##########
@@ -200,7 +124,7 @@ trait CalculatedPrefixedLengthUnparserMixin {
UnparseError(
One(state.schemaFileLocation),
One(state.currentLocation),
- s"The calculated value of ${elem.namedQName} ($adjustedLenInUnits)
failed check due to ${check.errMsg}"
+ s"The calculated value of ${plElem.namedQName} ($adjustedLenInUnits)
failed check due to ${check.errMsg}"
Review Comment:
Is switching to `plElem` correct? Although the check is done against
`plElem`, `plElem` is really an ephemeral quasi-element that doesn't exist in
the real infoset, so it might be confusing to mention it in an error message.
Maybe this instead wants to be something like "The calculated prefix length of
${elem.namedQName} ..."?
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/runtime1/ElementBaseRuntime1Mixin.scala:
##########
@@ -65,7 +65,8 @@ trait ElementBaseRuntime1Mixin { self: ElementBase =>
// no reason (unless it is referenced in a contentLength expression).
val mightHaveSuspensions = (maybeFixedLengthInBits.isDefined &&
couldHaveSuspensions)
- isReferenced || mightHaveSuspensions
+ // we want to capture contentlength when LK = prefixed
Review Comment:
I think we should explain why we need to capture content length. Maybe
something like
> Some prefixed length unparsers are unable to calculate the prefixed length
of the field. Instead, they unparse the field to a buffer and the captured
content length of the buffer is used. For this reason, prefixed length elements
must capture content length for unparse.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
