You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "stevedlawrence (via GitHub)" <gi...@apache.org> on 2025/02/04 13:22:50 UTC
Re: [PR] Fix Bug with valueLength being overwritten after Trim [daffodil]
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org