Fix operator precedence handling across target languages

- Adds proper operator precedence tables for each target language and updates comparison operator handling to respect precedence rules. - Fixes issues with bitwise operations and comparisons in Go, Lua, Perl, Ruby and other languages.
kaitai-io · Dec 16, 2024 · 2c45ea6 · 2c45ea6
1 parent 542b241
commit 2c45ea6
Show file tree

Hide file tree

Showing 10 changed files with 210 additions and 15 deletions.
diff --git a/jvm/src/test/scala/io/kaitai/struct/translators/TranslatorSpec.scala b/jvm/src/test/scala/io/kaitai/struct/translators/TranslatorSpec.scala
@@ -159,9 +159,17 @@ class TranslatorSpec extends AnyFunSpec {
       PythonCompiler -> "3 - ((1 + 2) // (7 * 8) + 5)"
     ))
 
-    everybody("1 + 2 << 5", "1 + 2 << 5")
-    everybody("(1 + 2) << 5", "1 + 2 << 5")
-    everybody("1 + (2 << 5)", "1 + (2 << 5)")
+    everybodyExcept("1 + 2 << 5", "1 + 2 << 5", ResultMap(
+      GoCompiler -> "(1 + 2) << 5"
+    ))
+
+    everybodyExcept("(1 + 2) << 5", "1 + 2 << 5", ResultMap(
+      GoCompiler -> "(1 + 2) << 5"
+    ))
+
+    everybodyExcept("1 + (2 << 5)", "1 + (2 << 5)", ResultMap(
+      GoCompiler -> "1 + 2 << 5"
+    ))
 
     everybodyExcept("~777", "~777", ResultMap(
       GoCompiler -> "^777"
@@ -175,11 +183,26 @@ class TranslatorSpec extends AnyFunSpec {
     everybodyExcept("(0b01 & 0b10) >> 1", "(1 & 2) >> 1", ResultMap(
       JavaScriptCompiler -> "(1 & 2) >>> 1"
     ))
+
+    everybodyExcept("3 | 1 ^ 2", "3 | 1 ^ 2", ResultMap(
+      GoCompiler -> "3 | (1 ^ 2)",
+      LuaCompiler -> "3 | 1 ~ 2",
+      PerlCompiler -> "3 | (1 ^ 2)",
+      RubyCompiler -> "3 | (1 ^ 2)"
+    ))
   }
 
   describe("integer comparisons") {
     everybody("1 < 2", "1 < 2", CalcBooleanType)
     everybody("1 == 2", "1 == 2", CalcBooleanType)
+
+    everybodyExcept("100 & 1 == 0", "(100 & 1) == 0", ResultMap(
+      GoCompiler -> "100 & 1 == 0",
+      LuaCompiler -> "100 & 1 == 0",
+      PythonCompiler -> "100 & 1 == 0",
+      RubyCompiler -> "100 & 1 == 0",
+      RustCompiler -> "100 & 1 == 0",
+    ), CalcBooleanType)
   }
 
   describe("integer method calls") {

diff --git a/shared/src/main/scala/io/kaitai/struct/exprlang/Ast.scala b/shared/src/main/scala/io/kaitai/struct/exprlang/Ast.scala
@@ -106,7 +106,9 @@ object Ast {
     case object Or extends boolop
   }
 
-  sealed trait operator
+  trait binaryop
+
+  sealed trait operator extends binaryop
   case object operator {
     case object Add extends operator
     case object Sub  extends operator
@@ -131,7 +133,7 @@ object Ast {
     case object Minus extends unaryop
   }
 
-  sealed trait cmpop
+  sealed trait cmpop extends binaryop
   object cmpop {
     case object Eq extends cmpop
     case object NotEq extends cmpop

diff --git a/shared/src/main/scala/io/kaitai/struct/translators/CommonOps.scala b/shared/src/main/scala/io/kaitai/struct/translators/CommonOps.scala
@@ -7,23 +7,34 @@ trait CommonOps extends AbstractTranslator {
    * Provides operator precedence table, used for deciding whether
    * parenthesis guarding expression are necessary or not.
    *
-   * This is the default table, based on Python operator precedence model.
+   * This is the default table, based on c++ operator precedence model.
    * This is good enough for most C-like languages. Individual languages'
    * translators can override it if and when necessary to alter behavior.
    *
-   * @see https://docs.python.org/3/reference/expressions.html#operator-precedence
+   * @see https://en.cppreference.com/w/cpp/language/operator_precedence
+   * @see https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/
+   * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_precedence
+   * @see https://docs.oracle.com/javase/tutorial/java/nutsandbolts/operators.html
+   * @see https://www.php.net/manual/en/language.operators.precedence.php
    */
-  val OPERATOR_PRECEDENCE = Map[Ast.operator, Int](
+
+  val OPERATOR_PRECEDENCE = Map[Ast.binaryop, Int](
     Ast.operator.Mult -> 130,
     Ast.operator.Div -> 130,
     Ast.operator.Mod -> 130,
     Ast.operator.Add -> 120,
     Ast.operator.Sub -> 120,
     Ast.operator.LShift -> 110,
     Ast.operator.RShift -> 110,
-    Ast.operator.BitAnd -> 100,
-    Ast.operator.BitXor -> 90,
-    Ast.operator.BitOr -> 80,
+    Ast.cmpop.Lt -> 100,
+    Ast.cmpop.LtE -> 100,
+    Ast.cmpop.Gt -> 100,
+    Ast.cmpop.GtE -> 100,
+    Ast.cmpop.Eq -> 90,
+    Ast.cmpop.NotEq -> 90,
+    Ast.operator.BitAnd -> 80,
+    Ast.operator.BitXor -> 70,
+    Ast.operator.BitOr -> 60
   )
 
   def genericBinOp(left: Ast.expr, op: Ast.operator, right: Ast.expr, extPrec: Int): String =
@@ -55,17 +66,22 @@ trait CommonOps extends AbstractTranslator {
     }
   }
 
+  def doCompareOp(left: Ast.expr, op: Ast.cmpop, right: Ast.expr): String = {
+    val thisPrec = OPERATOR_PRECEDENCE(op)
+    s"${translate(left, thisPrec)} ${cmpOp(op)} ${translate(right, thisPrec)}"
+  }
+
   def doNumericCompareOp(left: Ast.expr, op: Ast.cmpop, right: Ast.expr): String =
-    s"${translate(left)} ${cmpOp(op)} ${translate(right)}"
+    doCompareOp(left, op, right)
 
   def doStrCompareOp(left: Ast.expr, op: Ast.cmpop, right: Ast.expr): String =
-    s"${translate(left)} ${cmpOp(op)} ${translate(right)}"
+    doCompareOp(left, op, right)
 
   def doEnumCompareOp(left: Ast.expr, op: Ast.cmpop, right: Ast.expr): String =
-    s"${translate(left)} ${cmpOp(op)} ${translate(right)}"
+    doCompareOp(left, op, right)
 
   def doBytesCompareOp(left: Ast.expr, op: Ast.cmpop, right: Ast.expr): String =
-    s"${translate(left)} ${cmpOp(op)} ${translate(right)}"
+    doCompareOp(left, op, right)
 
   def cmpOp(op: Ast.cmpop): String = {
     op match {

diff --git a/shared/src/main/scala/io/kaitai/struct/translators/GoTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/GoTranslator.scala
@@ -28,6 +28,28 @@ class GoTranslator(out: StringLanguageOutputWriter, provider: TypeProvider, impo
 
   var returnRes: Option[String] = None
 
+  /**
+  * @see https://go.dev/ref/spec#Operator_precedence
+  */
+  override val OPERATOR_PRECEDENCE = Map[Ast.binaryop, Int](
+    Ast.operator.Mult -> 130,
+    Ast.operator.Div -> 130,
+    Ast.operator.Mod -> 130,
+    Ast.operator.LShift -> 130,
+    Ast.operator.RShift -> 130,
+    Ast.operator.BitAnd -> 130,
+    Ast.operator.Add -> 120,
+    Ast.operator.Sub -> 120,
+    Ast.operator.BitXor -> 120,
+    Ast.operator.BitOr -> 120,
+    Ast.cmpop.Lt -> 110,
+    Ast.cmpop.LtE -> 110,
+    Ast.cmpop.Gt -> 110,
+    Ast.cmpop.GtE -> 110,
+    Ast.cmpop.Eq -> 110,
+    Ast.cmpop.NotEq -> 110
+  )
+
   override def translate(v: Ast.expr, extPrec: Int): String = resToStr(translateExpr(v, extPrec))
 
   def resToStr(r: TranslatorResult): String = r match {

diff --git a/shared/src/main/scala/io/kaitai/struct/translators/LuaTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/LuaTranslator.scala
@@ -10,6 +10,28 @@ import io.kaitai.struct.Utils
 
 class LuaTranslator(provider: TypeProvider, importList: ImportList) extends BaseTranslator(provider)
     with MinSignedIntegers {
+  /**
+  * @see https://www.lua.org/manual/5.4/manual.html#3.4.8
+  */
+  override val OPERATOR_PRECEDENCE = Map[Ast.binaryop, Int](
+    Ast.operator.Mult -> 130,
+    Ast.operator.Div -> 130,
+    Ast.operator.Mod -> 130,
+    Ast.operator.Add -> 120,
+    Ast.operator.Sub -> 120,
+    Ast.operator.LShift -> 110,
+    Ast.operator.RShift -> 110,
+    Ast.operator.BitAnd -> 100,
+    Ast.operator.BitXor -> 90,
+    Ast.operator.BitOr -> 80,
+    Ast.cmpop.Lt -> 70,
+    Ast.cmpop.LtE -> 70,
+    Ast.cmpop.Gt -> 70,
+    Ast.cmpop.GtE -> 70,
+    Ast.cmpop.Eq -> 70,
+    Ast.cmpop.NotEq -> 70
+  )
+
   override def doIntLiteral(n: BigInt): String = {
     if (n > Long.MaxValue && n <= Utils.MAX_UINT64) {
       // See <https://www.lua.org/manual/5.4/manual.html#3.1>:

diff --git a/shared/src/main/scala/io/kaitai/struct/translators/NimTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/NimTranslator.scala
@@ -10,6 +10,29 @@ import io.kaitai.struct.format.{EnumSpec, Identifier}
 import io.kaitai.struct.languages.NimCompiler.{ksToNim, namespaced, camelCase}
 
 class NimTranslator(provider: TypeProvider, importList: ImportList) extends BaseTranslator(provider) {
+  /**
+  * @see https://nim-lang.org/docs/manual.html#syntax-precedence
+  */
+  override val OPERATOR_PRECEDENCE = Map[Ast.binaryop, Int](
+    Ast.operator.Mult -> 130,
+    Ast.operator.Div -> 130,
+    Ast.operator.Mod -> 130,
+    Ast.operator.LShift -> 120,
+    Ast.operator.RShift -> 120,
+    Ast.operator.Add -> 110,
+    Ast.operator.Sub -> 110,
+    Ast.cmpop.Lt -> 100,
+    Ast.cmpop.LtE -> 100,
+    Ast.cmpop.Gt -> 100,
+    Ast.cmpop.GtE -> 100,
+    Ast.cmpop.Eq -> 100,
+    Ast.cmpop.NotEq -> 100,
+    Ast.operator.BitAnd -> 90,
+    Ast.operator.BitXor -> 80,
+    Ast.operator.BitOr -> 80
+
+  )
+
   // Members declared in io.kaitai.struct.translators.BaseTranslator
   override def bytesToStr(bytesExpr: String, encoding: String): String = {
     s"""encode($bytesExpr, ${doStringLiteral(encoding)})"""

diff --git a/shared/src/main/scala/io/kaitai/struct/translators/PerlTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/PerlTranslator.scala
@@ -8,6 +8,27 @@ import io.kaitai.struct.format.{EnumSpec, Identifier}
 import io.kaitai.struct.languages.PerlCompiler
 
 class PerlTranslator(provider: TypeProvider, importList: ImportList) extends BaseTranslator(provider) {
+  /**
+  * @see https://perldoc.perl.org/perlop#Operator-Precedence-and-Associativity
+  */
+  override val OPERATOR_PRECEDENCE = Map[Ast.binaryop, Int](
+    Ast.operator.Mult -> 130,
+    Ast.operator.Div -> 130,
+    Ast.operator.Mod -> 130,
+    Ast.operator.Add -> 120,
+    Ast.operator.Sub -> 120,
+    Ast.operator.LShift -> 110,
+    Ast.operator.RShift -> 110,
+    Ast.cmpop.Lt -> 100,
+    Ast.cmpop.LtE -> 100,
+    Ast.cmpop.Gt -> 100,
+    Ast.cmpop.GtE -> 100,
+    Ast.cmpop.Eq -> 90,
+    Ast.cmpop.NotEq -> 90,
+    Ast.operator.BitAnd -> 80,
+    Ast.operator.BitXor -> 70,
+    Ast.operator.BitOr -> 70
+  )
   // http://perldoc.perl.org/perlrebackslash.html#Character-Escapes
   override val asciiCharQuoteMap: Map[Char, String] = Map(
     '\t' -> "\\t",

diff --git a/shared/src/main/scala/io/kaitai/struct/translators/PythonTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/PythonTranslator.scala
@@ -7,6 +7,28 @@ import io.kaitai.struct.format.{EnumSpec, Identifier}
 import io.kaitai.struct.languages.{PythonCompiler, RubyCompiler}
 
 class PythonTranslator(provider: TypeProvider, importList: ImportList, config: RuntimeConfig) extends BaseTranslator(provider) {
+  /**
+  * @see https://docs.python.org/3/reference/expressions.html#operator-precedence
+  */
+  override val OPERATOR_PRECEDENCE = Map[Ast.binaryop, Int](
+    Ast.operator.Mult -> 130,
+    Ast.operator.Div -> 130,
+    Ast.operator.Mod -> 130,
+    Ast.operator.Add -> 120,
+    Ast.operator.Sub -> 120,
+    Ast.operator.LShift -> 110,
+    Ast.operator.RShift -> 110,
+    Ast.operator.BitAnd -> 100,
+    Ast.operator.BitXor -> 90,
+    Ast.operator.BitOr -> 80,
+    Ast.cmpop.Lt -> 70,
+    Ast.cmpop.LtE -> 70,
+    Ast.cmpop.Gt -> 70,
+    Ast.cmpop.GtE -> 70,
+    Ast.cmpop.Eq -> 70,
+    Ast.cmpop.NotEq -> 70
+  )
+
   override def genericBinOp(left: Ast.expr, op: Ast.operator, right: Ast.expr, extPrec: Int) = {
     (detectType(left), detectType(right), op) match {
       case (_: IntType, _: IntType, Ast.operator.Div) =>

diff --git a/shared/src/main/scala/io/kaitai/struct/translators/RubyTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/RubyTranslator.scala
@@ -8,6 +8,28 @@ import io.kaitai.struct.languages.RubyCompiler
 
 class RubyTranslator(provider: TypeProvider) extends BaseTranslator(provider)
   with ByteArraysAsTrueArrays[String] {
+  /**
+  * @see https://ruby-doc.org/core-2.6.2/doc/syntax/precedence_rdoc.html
+  */
+  override val OPERATOR_PRECEDENCE = Map[Ast.binaryop, Int](
+    Ast.operator.Mult -> 130,
+    Ast.operator.Div -> 130,
+    Ast.operator.Mod -> 130,
+    Ast.operator.Add -> 120,
+    Ast.operator.Sub -> 120,
+    Ast.operator.LShift -> 110,
+    Ast.operator.RShift -> 110,
+    Ast.operator.BitAnd -> 100,
+    Ast.operator.BitXor -> 90,
+    Ast.operator.BitOr -> 90,
+    Ast.cmpop.Lt -> 70,
+    Ast.cmpop.LtE -> 70,
+    Ast.cmpop.Gt -> 70,
+    Ast.cmpop.GtE -> 70,
+    Ast.cmpop.Eq -> 60,
+    Ast.cmpop.NotEq -> 60
+  )
+
   override def doByteArrayLiteral(arr: Seq[Byte]): String =
     s"[${arr.map(_ & 0xff).mkString(", ")}].pack('C*')"
   override def doByteArrayNonLiteral(elts: Seq[Ast.expr]): String =

diff --git a/shared/src/main/scala/io/kaitai/struct/translators/RustTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/RustTranslator.scala
@@ -16,6 +16,28 @@ class RustTranslator(provider: TypeProvider, config: RuntimeConfig)
 
   var lastFoundMemberClass: ClassSpec = provider.nowClass
 
+  /**
+  * @see https://doc.rust-lang.org/reference/expressions.html
+  */
+  override val OPERATOR_PRECEDENCE = Map[Ast.binaryop, Int](
+    Ast.operator.Mult -> 130,
+    Ast.operator.Div -> 130,
+    Ast.operator.Mod -> 130,
+    Ast.operator.Add -> 120,
+    Ast.operator.Sub -> 120,
+    Ast.operator.LShift -> 110,
+    Ast.operator.RShift -> 110,
+    Ast.operator.BitAnd -> 100,
+    Ast.operator.BitXor -> 90,
+    Ast.operator.BitOr -> 80,
+    Ast.cmpop.Lt -> 70,
+    Ast.cmpop.LtE -> 70,
+    Ast.cmpop.Gt -> 70,
+    Ast.cmpop.GtE -> 70,
+    Ast.cmpop.Eq -> 70,
+    Ast.cmpop.NotEq -> 70
+  )
+
   override def doByteArrayLiteral(arr: Seq[Byte]): String =
     "vec![" + arr.map(x => "%0#2xu8".format(x & 0xff)).mkString(", ") + "]"
   override def doByteArrayNonLiteral(elts: Seq[Ast.expr]): String =