fixup! fixup! Parse Error on Out of Range Binary Integers

- currently we pass in isSigned boolean from ElementBaseGrammarMixin, but we can use the new PrimType.PrimNumeric.isSigned member instead for simplicity. We also added the minWidth member to simplify checking the minimum width of PrimNumeric types

DAFFODIL-2339

daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/DaffodilCCodeGenerator.scala

@@ -278,7 +278,7 @@ object DaffodilCCodeGenerator
       case g: BinaryDouble => binaryFloatGenerateCode(g.e, lengthInBits = 64, cgState)
       case g: BinaryFloat => binaryFloatGenerateCode(g.e, lengthInBits = 32, cgState)
       case g: BinaryIntegerKnownLength =>
-        binaryIntegerKnownLengthGenerateCode(g.e, g.lengthInBits, g.signed, cgState)
+        binaryIntegerKnownLengthGenerateCode(g.e, g.lengthInBits, cgState)
       case g: CaptureContentLengthEnd => noop(g)
       case g: CaptureContentLengthStart => noop(g)
       case g: CaptureValueLengthEnd => noop(g)

daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/generators/BinaryIntegerKnownLengthCodeGenerator.scala

@@ -18,14 +18,14 @@
 package org.apache.daffodil.codegen.c.generators
 
 import org.apache.daffodil.core.dsom.ElementBase
+import org.apache.daffodil.runtime1.dpath.NodeInfo.PrimType
 
 trait BinaryIntegerKnownLengthCodeGenerator extends BinaryValueCodeGenerator {
 
   // Generate C code to parse and unparse an integer element
   def binaryIntegerKnownLengthGenerateCode(
     e: ElementBase,
     lengthInBits: Long,
-    signed: Boolean,
     cgState: CodeGeneratorState
   ): Unit = {
     val cLengthInBits = lengthInBits match {
@@ -35,6 +35,10 @@ trait BinaryIntegerKnownLengthCodeGenerator extends BinaryValueCodeGenerator {
       case n if n <= 64 => 64
       case _ => e.SDE("Binary integer lengths longer than 64 bits are not supported.")
     }
+    val signed = e.primType match {
+      case n: PrimType.PrimNumeric => n.isSigned
+      case _ => false
+    }
     val primType = if (signed) s"int$cLengthInBits" else s"uint$cLengthInBits"
     val addField = valueAddField(e, lengthInBits, primType, _, cgState)
     val validateFixed = valueValidateFixed(e, _, cgState)

daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ElementBase.scala

@@ -775,32 +775,36 @@ trait ElementBase
       result.isDefined && repElement.isSimpleType && representation == Representation.Binary
     ) {
       val nBits = result.get
-      if (isSignedIntegerType && nBits < 2) {
-        val outOfRangeStr =
-          "Minimum length for a signed binary integer is 2 bits, number of bits %d out of range. " +
-            "An unsigned integer with length 1 bit could be used instead."
-        if (tunable.allowSignedIntegerLength1Bit) {
-          SDW(
-            WarnID.SignedBinaryIntegerLength1Bit,
-            outOfRangeStr,
-            nBits
-          )
-        } else {
-          SDE(
-            outOfRangeStr,
-            nBits
-          )
-        }
-      }
-      schemaDefinitionWhen(
-        isUnsignedIntegerType && nBits < 1,
-        "Minimum length for an unsigned binary integer is 1 bit, number of bits %d out of range.",
-        nBits
-      )
       primType match {
         case primNumeric: NodeInfo.PrimType.PrimNumeric =>
-          if (primNumeric.width.isDefined) {
-            val width = primNumeric.width.get
+          if (primNumeric.minWidth.isDefined) {
+            val isSigned = primNumeric.isSigned
+            val signedStr = if (isSigned) "signed" else "unsigned"
+            val minWidth = primNumeric.minWidth.get
+            if(nBits < minWidth) {
+              val outOfRangeFmtStr =
+                "Minimum length for a %s binary integer is %d bit(s), number of bits %d out of range. " +
+                  "An unsigned integer with length 1 bit could be used instead."
+              if (isSigned && tunable.allowSignedIntegerLength1Bit) {
+                SDW(
+                  WarnID.SignedBinaryIntegerLength1Bit,
+                  outOfRangeFmtStr,
+                  signedStr,
+                  minWidth,
+                  nBits
+                )
+              } else {
+                SDE(
+                  outOfRangeFmtStr,
+                  signedStr,
+                  minWidth,
+                  nBits
+                )
+              }
+            }
+          }
+          if (primNumeric.maxWidth.isDefined) {
+            val width = primNumeric.maxWidth.get
             if (nBits > width) {
               SDE(
                 "Number of bits %d out of range for binary %s, must be between 1 and %d bits.",

daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala

@@ -782,7 +782,7 @@ trait ElementBaseGrammarMixin
   ) {
     ConvertZonedCombinator(
       this,
-      new IBM4690PackedIntegerKnownLength(this, false, binaryNumberKnownLengthInBits),
+      new IBM4690PackedIntegerKnownLength(this, binaryNumberKnownLengthInBits),
       textConverter
     )
   }
@@ -792,7 +792,7 @@ trait ElementBaseGrammarMixin
   ) {
     ConvertZonedCombinator(
       this,
-      new IBM4690PackedIntegerRuntimeLength(this, false),
+      new IBM4690PackedIntegerRuntimeLength(this),
       textConverter
     )
   }
@@ -802,7 +802,7 @@ trait ElementBaseGrammarMixin
   ) {
     ConvertZonedCombinator(
       this,
-      new IBM4690PackedIntegerDelimitedEndOfData(this, false),
+      new IBM4690PackedIntegerDelimitedEndOfData(this),
       textConverter
     )
   }
@@ -812,7 +812,7 @@ trait ElementBaseGrammarMixin
   ) {
     ConvertZonedCombinator(
       this,
-      new IBM4690PackedIntegerPrefixedLength(this, false),
+      new IBM4690PackedIntegerPrefixedLength(this),
       textConverter
     )
   }
@@ -823,7 +823,6 @@ trait ElementBaseGrammarMixin
         this,
         new PackedIntegerKnownLength(
           this,
-          false,
           packedSignCodes,
           binaryNumberKnownLengthInBits
         ),
@@ -834,23 +833,23 @@ trait ElementBaseGrammarMixin
     prod("packedRuntimeLengthCalendar", binaryCalendarRep == BinaryCalendarRep.Packed) {
       ConvertZonedCombinator(
         this,
-        new PackedIntegerRuntimeLength(this, false, packedSignCodes),
+        new PackedIntegerRuntimeLength(this, packedSignCodes),
         textConverter
       )
     }
   private lazy val packedDelimitedLengthCalendar =
     prod("packedDelimitedLengthCalendar", binaryCalendarRep == BinaryCalendarRep.Packed) {
       ConvertZonedCombinator(
         this,
-        new PackedIntegerDelimitedEndOfData(this, false, packedSignCodes),
+        new PackedIntegerDelimitedEndOfData(this, packedSignCodes),
         textConverter
       )
     }
   private lazy val packedPrefixedLengthCalendar =
     prod("packedPrefixedLengthCalendar", binaryCalendarRep == BinaryCalendarRep.Packed) {
       ConvertZonedCombinator(
         this,
-        new PackedIntegerPrefixedLength(this, false, packedSignCodes),
+        new PackedIntegerPrefixedLength(this, packedSignCodes),
         textConverter
       )
     }
@@ -893,7 +892,7 @@ trait ElementBaseGrammarMixin
   private lazy val packedSignCodes =
     PackedSignCodes(binaryPackedSignCodes, binaryNumberCheckPolicy)
 
-  private def binaryIntegerValue(isSigned: Boolean) = {
+  private def binaryIntegerValue = {
     //
     // Is it a single byte or smaller
     //
@@ -906,10 +905,10 @@ trait ElementBaseGrammarMixin
     }
     (binaryNumberRep, lengthKind, binaryNumberKnownLengthInBits) match {
       case (BinaryNumberRep.Binary, LengthKind.Prefixed, _) =>
-        new BinaryIntegerPrefixedLength(this, isSigned)
-      case (BinaryNumberRep.Binary, _, -1) => new BinaryIntegerRuntimeLength(this, isSigned)
+        new BinaryIntegerPrefixedLength(this)
+      case (BinaryNumberRep.Binary, _, -1) => new BinaryIntegerRuntimeLength(this)
       case (BinaryNumberRep.Binary, _, _) =>
-        new BinaryIntegerKnownLength(this, isSigned, binaryNumberKnownLengthInBits)
+        new BinaryIntegerKnownLength(this, binaryNumberKnownLengthInBits)
       case (_, LengthKind.Implicit, _) =>
         SDE("lengthKind='implicit' is not allowed with packed binary formats")
       case (_, _, _)
@@ -919,26 +918,25 @@ trait ElementBaseGrammarMixin
           binaryNumberKnownLengthInBits
         )
       case (BinaryNumberRep.Packed, LengthKind.Delimited, -1) =>
-        new PackedIntegerDelimitedEndOfData(this, isSigned, packedSignCodes)
+        new PackedIntegerDelimitedEndOfData(this, packedSignCodes)
       case (BinaryNumberRep.Packed, LengthKind.Prefixed, -1) =>
-        new PackedIntegerPrefixedLength(this, isSigned, packedSignCodes)
+        new PackedIntegerPrefixedLength(this, packedSignCodes)
       case (BinaryNumberRep.Packed, _, -1) =>
-        new PackedIntegerRuntimeLength(this, isSigned, packedSignCodes)
+        new PackedIntegerRuntimeLength(this, packedSignCodes)
       case (BinaryNumberRep.Packed, _, _) =>
         new PackedIntegerKnownLength(
           this,
-          isSigned,
           packedSignCodes,
           binaryNumberKnownLengthInBits
         )
       case (BinaryNumberRep.Ibm4690Packed, LengthKind.Delimited, -1) =>
-        new IBM4690PackedIntegerDelimitedEndOfData(this, isSigned)
+        new IBM4690PackedIntegerDelimitedEndOfData(this)
       case (BinaryNumberRep.Ibm4690Packed, LengthKind.Prefixed, -1) =>
-        new IBM4690PackedIntegerPrefixedLength(this, isSigned)
+        new IBM4690PackedIntegerPrefixedLength(this)
       case (BinaryNumberRep.Ibm4690Packed, _, -1) =>
-        new IBM4690PackedIntegerRuntimeLength(this, isSigned)
+        new IBM4690PackedIntegerRuntimeLength(this)
       case (BinaryNumberRep.Ibm4690Packed, _, _) =>
-        new IBM4690PackedIntegerKnownLength(this, isSigned, binaryNumberKnownLengthInBits)
+        new IBM4690PackedIntegerKnownLength(this, binaryNumberKnownLengthInBits)
       case (BinaryNumberRep.Bcd, _, _) =>
         primType match {
           case PrimType.Long | PrimType.Int | PrimType.Short | PrimType.Byte =>
@@ -954,19 +952,6 @@ trait ElementBaseGrammarMixin
     }
   }
 
-  lazy val isSignedIntegerType: Boolean = primType match {
-    case PrimType.Byte | PrimType.Short | PrimType.Int | PrimType.Long | PrimType.Integer =>
-      true
-    case _ => false
-  }
-
-  lazy val isUnsignedIntegerType: Boolean = primType match {
-    case PrimType.UnsignedByte | PrimType.UnsignedShort | PrimType.UnsignedInt |
-        PrimType.UnsignedLong | PrimType.NonNegativeInteger =>
-      true
-    case _ => false
-  }
-
   private lazy val binaryValue: Gram = {
     Assert.invariant(primType != PrimType.String)
 
@@ -983,12 +968,8 @@ trait ElementBaseGrammarMixin
     // This is in the spirit of that section.
     val res: Gram = primType match {
 
-      case _ if isSignedIntegerType => {
-        binaryIntegerValue(true)
-      }
-
-      case _ if isUnsignedIntegerType => {
-        binaryIntegerValue(false)
+      case n: PrimType.PrimNumeric if n.isInteger => {
+        binaryIntegerValue
       }
 
       case PrimType.Double | PrimType.Float => {

daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesBinaryNumber.scala

@@ -40,36 +40,34 @@ import org.apache.daffodil.unparsers.runtime1.BinaryIntegerKnownLengthUnparser
 import org.apache.daffodil.unparsers.runtime1.BinaryIntegerPrefixedLengthUnparser
 import org.apache.daffodil.unparsers.runtime1.BinaryIntegerRuntimeLengthUnparser
 
-class BinaryIntegerRuntimeLength(val e: ElementBase, signed: Boolean)
+class BinaryIntegerRuntimeLength(val e: ElementBase)
   extends Terminal(e, true) {
 
   override lazy val parser = new BinaryIntegerRuntimeLengthParser(
     e.elementRuntimeData,
-    signed,
     e.lengthEv,
     e.lengthUnits
   )
 
   override lazy val unparser: Unparser = new BinaryIntegerRuntimeLengthUnparser(
     e.elementRuntimeData,
-    signed,
     e.lengthEv,
     e.lengthUnits
   )
 }
 
-class BinaryIntegerKnownLength(val e: ElementBase, val signed: Boolean, val lengthInBits: Long)
+class BinaryIntegerKnownLength(val e: ElementBase, val lengthInBits: Long)
   extends Terminal(e, true) {
 
   override lazy val parser = {
-    new BinaryIntegerKnownLengthParser(e.elementRuntimeData, signed, lengthInBits.toInt)
+    new BinaryIntegerKnownLengthParser(e.elementRuntimeData, lengthInBits.toInt)
   }
 
   override lazy val unparser: Unparser =
-    new BinaryIntegerKnownLengthUnparser(e.elementRuntimeData, signed, lengthInBits.toInt)
+    new BinaryIntegerKnownLengthUnparser(e.elementRuntimeData, lengthInBits.toInt)
 }
 
-class BinaryIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
+class BinaryIntegerPrefixedLength(val e: ElementBase)
   extends Terminal(e, true) {
 
   private lazy val erd = e.elementRuntimeData
@@ -81,7 +79,6 @@ class BinaryIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
       erd,
       e.prefixedLengthBody.parser,
       plerd,
-      signed,
       e.lengthUnits,
       pladj
     )
@@ -101,7 +98,6 @@ class BinaryIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
       e.prefixedLengthBody.unparser,
       plerd,
       maybeNBits,
-      signed,
       e.lengthUnits,
       pladj
     )

daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesIBM4690Packed.scala

@@ -33,11 +33,10 @@ import org.apache.daffodil.unparsers.runtime1.IBM4690PackedIntegerKnownLengthUnp
 import org.apache.daffodil.unparsers.runtime1.IBM4690PackedIntegerPrefixedLengthUnparser
 import org.apache.daffodil.unparsers.runtime1.IBM4690PackedIntegerRuntimeLengthUnparser
 
-class IBM4690PackedIntegerRuntimeLength(val e: ElementBase, signed: Boolean)
+class IBM4690PackedIntegerRuntimeLength(val e: ElementBase)
   extends Terminal(e, true) {
   override lazy val parser = new IBM4690PackedIntegerRuntimeLengthParser(
     e.elementRuntimeData,
-    signed,
     e.lengthEv,
     e.lengthUnits
   )
@@ -49,23 +48,22 @@ class IBM4690PackedIntegerRuntimeLength(val e: ElementBase, signed: Boolean)
   )
 }
 
-class IBM4690PackedIntegerKnownLength(val e: ElementBase, signed: Boolean, lengthInBits: Long)
+class IBM4690PackedIntegerKnownLength(val e: ElementBase, lengthInBits: Long)
   extends Terminal(e, true) {
 
   override lazy val parser =
-    new IBM4690PackedIntegerKnownLengthParser(e.elementRuntimeData, signed, lengthInBits.toInt)
+    new IBM4690PackedIntegerKnownLengthParser(e.elementRuntimeData, lengthInBits.toInt)
 
   override lazy val unparser: Unparser =
     new IBM4690PackedIntegerKnownLengthUnparser(e.elementRuntimeData, lengthInBits.toInt)
 }
 
-class IBM4690PackedIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
+class IBM4690PackedIntegerPrefixedLength(val e: ElementBase)
   extends Terminal(e, true) {
   override lazy val parser = new IBM4690PackedIntegerPrefixedLengthParser(
     e.elementRuntimeData,
     e.prefixedLengthBody.parser,
     e.prefixedLengthElementDecl.elementRuntimeData,
-    signed,
     e.lengthUnits,
     e.prefixedLengthAdjustmentInUnits
   )

daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesLengthKind.scala

@@ -205,7 +205,6 @@ case class HexBinaryLengthPrefixed(e: ElementBase) extends Terminal(e, true) {
 
 abstract class PackedIntegerDelimited(
   e: ElementBase,
-  signed: Boolean,
   packedSignCodes: PackedSignCodes
 ) extends StringDelimited(e) {
 
@@ -223,9 +222,8 @@ abstract class PackedIntegerDelimited(
 
 case class PackedIntegerDelimitedEndOfData(
   e: ElementBase,
-  signed: Boolean,
   packedSignCodes: PackedSignCodes
-) extends PackedIntegerDelimited(e, signed, packedSignCodes) {
+) extends PackedIntegerDelimited(e, packedSignCodes) {
   val isDelimRequired: Boolean = false
 }
 
@@ -289,7 +287,7 @@ case class BCDDecimalDelimitedEndOfData(e: ElementBase) extends BCDDecimalDelimi
   val isDelimRequired: Boolean = false
 }
 
-abstract class IBM4690PackedIntegerDelimited(e: ElementBase, signed: Boolean)
+abstract class IBM4690PackedIntegerDelimited(e: ElementBase)
   extends StringDelimited(e) {
 
   override lazy val parser: DaffodilParser = new IBM4690PackedIntegerDelimitedParser(
@@ -304,8 +302,8 @@ abstract class IBM4690PackedIntegerDelimited(e: ElementBase, signed: Boolean)
   )
 }
 
-case class IBM4690PackedIntegerDelimitedEndOfData(e: ElementBase, signed: Boolean)
-  extends IBM4690PackedIntegerDelimited(e, signed) {
+case class IBM4690PackedIntegerDelimitedEndOfData(e: ElementBase)
+  extends IBM4690PackedIntegerDelimited(e) {
   val isDelimRequired: Boolean = false
 }