Implement new enums syntax (enforced underlying type, doc / doc-ref support)

shared/src/main/scala/io/kaitai/struct/ClassCompiler.scala

@@ -487,7 +487,7 @@ class ClassCompiler(
   }
 
   def compileEnum(curClass: ClassSpec, enumColl: EnumSpec): Unit =
-    lang.enumDeclaration(curClass.name, enumColl.name.last, enumColl.map.toSeq)
+    lang.enumDeclaration(curClass.name, enumColl.name.last, enumColl.map.toSeq, enumColl)
 
   def compileClassDoc(curClass: ClassSpec): Unit = {
     if (!curClass.doc.isEmpty)

shared/src/main/scala/io/kaitai/struct/RuntimeConfig.scala

@@ -10,13 +10,22 @@ package io.kaitai.struct
   * @param useListInitializers If true, allows use of list initializers for
   *                            `std::vector` and `std::set`. Otherwise, throw a fatal
   *                            "not implemented" error.
+  * @param enumsFixedUnderlyingType If true, enums will be declared with fixed
+  * underlying type (see https://github.com/kaitai-io/kaitai_struct/issues/1288).
+  * If false, enums will be declared without a fixed underlying type. Enums without
+  * a fixed underlying type are best avoided, because casting an integer that falls
+  * outside the range of the enum values to the enum type is undefined behavior (see
+  * https://github.com/kaitai-io/kaitai_struct/issues/959). However, in C++98, this
+  * is the only supported option (see
+  * https://cppreference.com/cpp/language/enum#Unscoped_enumerations).
   * @param pointers Choose which style of pointers to use.
   */
 case class CppRuntimeConfig(
   namespace: List[String] = List(),
   usePragmaOnce: Boolean = false,
   stdStringFrontBack: Boolean = false,
   useListInitializers: Boolean = false,
+  enumsFixedUnderlyingType: Boolean = false,
   pointers: CppRuntimeConfig.Pointers = CppRuntimeConfig.RawPointers
 ) {
   /**
@@ -27,6 +36,7 @@ case class CppRuntimeConfig(
     usePragmaOnce = false,
     stdStringFrontBack = false,
     useListInitializers = false,
+    enumsFixedUnderlyingType = false,
     pointers = CppRuntimeConfig.RawPointers
   )
 
@@ -38,6 +48,7 @@ case class CppRuntimeConfig(
     usePragmaOnce = true,
     stdStringFrontBack = true,
     useListInitializers = true,
+    enumsFixedUnderlyingType = true,
     pointers = CppRuntimeConfig.UniqueAndRawPointers
   )
 }

shared/src/main/scala/io/kaitai/struct/datatype/DataType.scala

@@ -45,20 +45,90 @@ object DataType {
   abstract sealed class NumericType extends DataType
   abstract sealed class BooleanType extends DataType
 
-  abstract sealed class IntType extends NumericType
-  case object CalcIntType extends IntType
+  abstract sealed class IntType extends NumericType {
+    /**
+      * The smallest value of the given integer type
+      */
+    def min: BigInt
+    /**
+      * The largest value of the given integer type
+      */
+    def max: BigInt
+    /**
+      * Render the integer type as a KSY-style "pure" type string (e.g. `u4`,
+      * `s1`, `b3`) without any serialization details, such as endianness.
+      * Intended for use in error messages.
+      */
+    def toPureTypeString: String
+
+    final def subsetOf(other: IntType): Boolean = {
+      // This implementation might be slightly inefficient (compared to pattern
+      // matching with a few simple comparisons) because it involves calculating
+      // large `BigInt` values, but it's guaranteed to give the correct result.
+      // That's because it directly tests the "subset of" relationship by
+      // comparing the `min` and `max` bounds.
+      min >= other.min && max <= other.max
+    }
+  }
+  object IntType {
+    final val NUM_BITS_IN_BYTE = 8
+  }
+  /**
+    * In statically typed languages, [[CalcIntType]] is often a signed 32-bit
+    * integer, but from the compiler's perspective, it represents an unspecified
+    * integer type. Therefore, we deliberately don't implement the [[min]] and
+    * [[max]] methods.
+    */
+  case object CalcIntType extends IntType {
+    override final def min: BigInt = ???
+    override final def max: BigInt = ???
+    override def toPureTypeString: String = "<calc>"
+  }
   case class Int1Type(signed: Boolean) extends IntType with ReadableType {
+    override final def min: BigInt =
+      if (signed) {
+        Byte.MinValue
+      } else {
+        0
+      }
+    override final def max: BigInt =
+      if (signed) {
+        Byte.MaxValue
+      } else {
+        0xff
+      }
+    override def toPureTypeString: String = if (signed) "s1" else "u1"
     override def apiCall(defEndian: Option[FixedEndian]): String = if (signed) "s1" else "u1"
   }
   case class IntMultiType(signed: Boolean, width: IntWidth, endian: Option[FixedEndian]) extends IntType with ReadableType {
+    private final def bitWidth: Int =
+      IntType.NUM_BITS_IN_BYTE * width.width
+
+    override final def min: BigInt =
+      if (signed) {
+        -(BigInt(1) << (bitWidth - 1))
+      } else {
+        0
+      }
+    override final def max: BigInt =
+      (BigInt(1) << (bitWidth - (if (signed) 1 else 0))) - 1
+    override def toPureTypeString: String =
+      s"${if (signed) 's' else 'u'}${width.width}"
+
     override def apiCall(defEndian: Option[FixedEndian]): String = {
       val ch1 = if (signed) 's' else 'u'
       val finalEnd = endian.orElse(defEndian)
       s"$ch1${width.width}${finalEnd.map(_.toSuffix).getOrElse("")}"
     }
   }
   case class BitsType1(bitEndian: BitEndianness) extends BooleanType
-  case class BitsType(width: Int, bitEndian: BitEndianness) extends IntType
+  case class BitsType(width: Int, bitEndian: BitEndianness) extends IntType {
+    override final def min: BigInt = 0
+    override final def max: BigInt =
+      (BigInt(1) << width) - 1
+    override def toPureTypeString: String =
+      s"b$width"
+  }
 
   abstract class FloatType extends NumericType
   case object CalcFloatType extends FloatType

shared/src/main/scala/io/kaitai/struct/format/EnumSpec.scala

@@ -4,8 +4,19 @@ import io.kaitai.struct.problems.KSYParseError
 
 import scala.collection.immutable.SortedMap
 import scala.collection.mutable
+import io.kaitai.struct.Utils
+import io.kaitai.struct.datatype.DataType.IntType
+import io.kaitai.struct.datatype.DataType
+import io.kaitai.struct.datatype.DataType.BitsType1
+import io.kaitai.struct.datatype.DataType.BitsType
+import scala.util.control.NonFatal
 
-case class EnumSpec(path: List[String], map: SortedMap[BigInt, EnumValueSpec]) extends YAMLPath {
+case class EnumSpec(
+  path: List[String],
+  doc: DocSpec,
+  intType: IntType,
+  map: SortedMap[BigInt, EnumValueSpec]
+) extends YAMLPath {
   var name = List[String]()
 
   /**
@@ -26,18 +37,80 @@ case class EnumSpec(path: List[String], map: SortedMap[BigInt, EnumValueSpec]) e
 }
 
 object EnumSpec {
+  val LEGAL_KEYS = Set(
+    "doc",
+    "doc-ref",
+    "type",
+    "values"
+  )
+
   def fromYaml(src: Any, path: List[String]): EnumSpec = {
     val srcMap = ParseUtils.asMap(src, path)
+    // Check whether we're dealing with the old enum syntax used in KS 0.11 and
+    // earlier. Strictly speaking, this is not necessary (the old syntax would
+    // be rejected anyway), but we do this to make the error message as helpful
+    // as possible.
+    if (srcMap.nonEmpty && !LEGAL_KEYS.exists(srcMap.contains) && srcMap.exists { case (key, _) =>
+      try {
+        ParseUtils.asBigInt(key, Nil)
+        true
+      } catch {
+        case NonFatal(_) => false
+      }
+    }) {
+      throw KSYParseError.withText(
+        "legacy pre-v0.12 enum syntax; add `type: <int_type>` (e.g. `type: u4`) and indent entries under the `values` key",
+        path
+      )
+    }
+
+    // At this point, either the map is empty or contains no integer keys (in
+    // which case we will report missing mandatory properties of the new
+    // syntax), or an attempt was made to use the new KS 0.12+ syntax (see
+    // https://github.com/kaitai-io/kaitai_struct/issues/1288), so we are
+    // treating it as such
+    val srcMapStr = ParseUtils.anyMapToStrMap(srcMap, path)
+    ParseUtils.ensureLegalKeys(srcMapStr, LEGAL_KEYS, path)
+
+    val typeStr = ParseUtils.getValueStr(srcMapStr, "type", path)
+    val valuesMap = ParseUtils.getValueMap(srcMapStr, "values", path)
+
+    val dataType = DataType.pureFromString(Some(typeStr))
+    val intType = dataType match {
+      case it: IntType => it
+      // `type: b1` is automatically mapped to a boolean by
+      // DataType.pureFromString(). We don't want that here, so we convert it
+      // back to a 1-bit integer.
+      case BitsType1(bitEndian) =>
+        BitsType(1, bitEndian)
+      case other =>
+        throw KSYParseError.withText(
+          s"expected an integer type with no endianness (i.e. `uX` / `sX` / `bX`), got `${typeStr}`",
+          path :+ "type"
+        )
+    }
+    val intTypeMin = intType.min
+    val intTypeMax = intType.max
+
+    val doc = DocSpec.fromYaml(srcMapStr, path)
+
     val memberNameMap = mutable.Map[String, BigInt]()
-    EnumSpec(path, SortedMap.from(
-      srcMap.map { case (id, desc) =>
-        val idBigInt = ParseUtils.asBigInt(id, path)
-        val value = EnumValueSpec.fromYaml(desc, path ++ List(idBigInt.toString))
+    val valuesPath = path :+ "values"
 
+    EnumSpec(path, doc, intType, SortedMap.from(
+      valuesMap.map { case (id, desc) =>
+        val idBigInt = ParseUtils.asBigInt(id, valuesPath)
+        if (idBigInt < intTypeMin || idBigInt > intTypeMax) {
+          throw KSYParseError.withText(
+            s"integer constant ${idBigInt} is out of range ${intTypeMin}..${intTypeMax} of the enum's underlying type `${typeStr}`",
+            valuesPath :+ idBigInt.toString
+          )
+        }
+        val value = EnumValueSpec.fromYaml(desc, valuesPath :+ idBigInt.toString)
         memberNameMap.get(value.name).foreach { (prevIdBigInt) =>
           throw KSYParseError.withText(
-            s"duplicate enum member ID: '${value.name}', previously defined at /${(path ++ List(prevIdBigInt.toString)).mkString("/")}",
-            path ++ List(idBigInt.toString)
+            s"duplicate enum member ID: '${value.name}', previously defined at /${(valuesPath :+ prevIdBigInt.toString).mkString("/")}",
+            valuesPath :+ idBigInt.toString
           )
         }
         memberNameMap.put(value.name, idBigInt)

shared/src/main/scala/io/kaitai/struct/format/ParseUtils.scala

@@ -39,6 +39,15 @@ object ParseUtils {
     }
   }
 
+  def getValueMap(src: Map[String, Any], field: String, path: List[String]): Map[Any, Any] = {
+    src.get(field) match {
+      case Some(value) =>
+        asMap(value, path ++ List(field))
+      case None =>
+        throw KSYParseError.noKey(field, path)
+    }
+  }
+
   def getOptValueStr(src: Map[String, Any], field: String, path: List[String]): Option[String] = {
     src.get(field) match {
       case None =>

shared/src/main/scala/io/kaitai/struct/languages/CSharpCompiler.scala

@@ -547,11 +547,13 @@ class CSharpCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
 
   def flagForInstName(ksName: Identifier) = s"f_${idToStr(ksName)}"
 
-  override def enumDeclaration(curClass: String, enumName: String, enumColl: Seq[(BigInt, String)]): Unit = {
+  override def enumDeclaration(curClass: String, enumName: String, enumColl: Seq[(BigInt, String)], enumSpec: EnumSpec): Unit = {
     val enumClass = type2class(enumName)
 
     out.puts
-    out.puts(s"public enum $enumClass")
+    if (!enumSpec.doc.isEmpty)
+      universalDoc(enumSpec.doc)
+    out.puts(s"public enum $enumClass : ${kaitaiType2NativeType(enumSpec.intType)}")
     out.puts(s"{")
     out.inc
 

shared/src/main/scala/io/kaitai/struct/languages/CppCompiler.scala

@@ -885,11 +885,17 @@ class CppCompiler(
     handleAssignmentSimple(instName, valExprConverted)
   }
 
-  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)]): Unit = {
+  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)], enumSpec: EnumSpec): Unit = {
     val enumClass = types2class(List(enumName))
 
     outHdr.puts
-    outHdr.puts(s"enum $enumClass {")
+    if (!enumSpec.doc.isEmpty)
+      universalDoc(enumSpec.doc)
+    if (config.cppConfig.enumsFixedUnderlyingType) {
+      outHdr.puts(s"enum $enumClass : ${kaitaiType2NativeType(enumSpec.intType)} {")
+    } else {
+      outHdr.puts(s"enum $enumClass {")
+    }
     outHdr.inc
 
     if (enumColl.size > 1) {

shared/src/main/scala/io/kaitai/struct/languages/GoCompiler.scala

@@ -491,12 +491,14 @@ class GoCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
   override def instanceSetCalculated(instName: InstanceIdentifier): Unit =
     out.puts(s"this.${calculatedFlagForName(instName)} = true")
 
-  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)]): Unit = {
+  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)], enumSpec: EnumSpec): Unit = {
     val fullEnumName: List[String] = curClass ++ List(enumName)
     val fullEnumNameStr = types2class(fullEnumName)
 
     out.puts
-    out.puts(s"type $fullEnumNameStr int")
+    if (!enumSpec.doc.isEmpty)
+      universalDoc(enumSpec.doc)
+    out.puts(s"type $fullEnumNameStr ${kaitaiType2NativeType(enumSpec.intType)}")
     out.puts("const (")
     out.inc
 

shared/src/main/scala/io/kaitai/struct/languages/JavaCompiler.scala

@@ -947,10 +947,12 @@ class JavaCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
     out.puts(s"public void _invalidate${idToSetterStr(instName)}() { ${privateMemberName(instName)} = null; }")
   }
 
-  override def enumDeclaration(curClass: String, enumName: String, enumColl: Seq[(BigInt, String)]): Unit = {
+  override def enumDeclaration(curClass: String, enumName: String, enumColl: Seq[(BigInt, String)], enumSpec: EnumSpec): Unit = {
     val enumClass = type2class(enumName)
 
     out.puts
+    if (!enumSpec.doc.isEmpty)
+      universalDoc(enumSpec.doc)
     out.puts(s"public enum $enumClass {")
     out.inc
 

shared/src/main/scala/io/kaitai/struct/languages/JavaScriptCompiler.scala

@@ -525,7 +525,9 @@ class JavaScriptCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
     out.puts(s"return ${privateMemberName(instName)};")
   }
 
-  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)]): Unit = {
+  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)], enumSpec: EnumSpec): Unit = {
+    if (!enumSpec.doc.isEmpty)
+      universalDoc(enumSpec.doc)
     out.puts(s"${type2class(curClass.last)}.${type2class(enumName)} = Object.freeze({")
     out.inc
 

shared/src/main/scala/io/kaitai/struct/languages/LuaCompiler.scala

@@ -273,9 +273,11 @@ class LuaCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
   override def instanceReturn(instName: InstanceIdentifier, attrType: DataType, isNullable: Boolean): Unit =
     out.puts(s"return ${privateMemberName(instName)}")
 
-  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)]): Unit = {
+  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)], enumSpec: EnumSpec): Unit = {
     importList.add("local enum = require(\"enum\")")
 
+    if (!enumSpec.doc.isEmpty)
+      universalDoc(enumSpec.doc)
     out.puts(s"${types2class(curClass)}.${type2class(enumName)} = enum.Enum {")
     out.inc
     enumColl.foreach { case (id, label) =>

shared/src/main/scala/io/kaitai/struct/languages/NimCompiler.scala

@@ -183,10 +183,12 @@ class NimCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
     out.dec
   }
   // For this to work, we need a {.lenientCase.} pragma which disables nim's exhaustive case coverage check
-  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)]): Unit = {
+  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)], enumSpec: EnumSpec): Unit = {
     val enumClass = namespaced(curClass)
     out.puts(s"${enumClass}_${camelCase(enumName, true)}* = enum")
     out.inc
+    if (!enumSpec.doc.isEmpty)
+      universalDoc(enumSpec.doc)
     enumColl.foreach { case (id, label) =>
       val order = if (s"$id" == "-9223372036854775808") "low(int64)" else s"$id"
       out.puts(s"${label.name} = $order")

shared/src/main/scala/io/kaitai/struct/languages/PHPCompiler.scala

@@ -436,8 +436,10 @@ class PHPCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
     out.puts(s"return ${privateMemberName(instName)};")
   }
 
-  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)]): Unit = {
+  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)], enumSpec: EnumSpec): Unit = {
     val name = curClass ::: List(enumName)
+    if (!enumSpec.doc.isEmpty)
+      universalDoc(enumSpec.doc)
     classHeader(name, None)
     enumColl.foreach { case (id, label) =>
       universalDoc(label.doc)

shared/src/main/scala/io/kaitai/struct/languages/PerlCompiler.scala

@@ -425,7 +425,7 @@ class PerlCompiler(typeProvider: ClassTypeProvider, config: RuntimeConfig)
     out.puts(s"return ${privateMemberName(instName)};")
   }
 
-  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)]): Unit = {
+  override def enumDeclaration(curClass: List[String], enumName: String, enumColl: Seq[(BigInt, EnumValueSpec)], enumSpec: EnumSpec): Unit = {
     out.puts
 
     enumColl.foreach { case (id, label) =>