olabusayoT commented on code in PR #1338:
URL: https://github.com/apache/daffodil/pull/1338#discussion_r1941729436
##########
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:
hmm...im surprised it has no coverage, several tests pass that condition of
isSimpleType and impliedRep == Text and isHexBinary and LK implicit!
ex: maxHexBinaryError, hexBinary_Implicit_03b etc
These tests fail when we remove that case
--
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]
