olabusayoT commented on code in PR #1338:
URL: https://github.com/apache/daffodil/pull/1338#discussion_r1941736405


##########
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:
   Maybe there's a bug with code coverage, hexBinary_bits_be_msbf definitely 
hits the default 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]

Reply via email to