From a28b2aa9cf80f85916c00bbd37209b015d3f1e4d Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Tue, 26 Mar 2024 15:31:26 +0000 Subject: [PATCH] Implement optimal parenthesis for ternary operator + add tests --- .../struct/translators/TranslatorSpec.scala | 97 +++++++++++++++++-- .../struct/translators/BaseTranslator.scala | 9 +- .../struct/translators/CSharpTranslator.scala | 2 - .../kaitai/struct/translators/CommonOps.scala | 14 +++ .../struct/translators/CppTranslator.scala | 4 +- .../translators/JavaScriptTranslator.scala | 2 - .../struct/translators/JavaTranslator.scala | 2 - .../struct/translators/LuaTranslator.scala | 2 +- .../struct/translators/NimTranslator.scala | 2 +- .../struct/translators/PHPTranslator.scala | 4 +- .../struct/translators/PerlTranslator.scala | 4 +- .../struct/translators/PythonTranslator.scala | 13 ++- .../struct/translators/RubyTranslator.scala | 2 - .../struct/translators/RustTranslator.scala | 2 +- 14 files changed, 125 insertions(+), 34 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 index 355bf975b..3910e326c 100644 --- a/jvm/src/test/scala/io/kaitai/struct/translators/TranslatorSpec.scala +++ b/jvm/src/test/scala/io/kaitai/struct/translators/TranslatorSpec.scala @@ -226,9 +226,50 @@ class TranslatorSpec extends AnyFunSpec { } describe("ternary operator") { + everybodyExcept("2 < 3 ? 123 : 456", "2 < 3 ? 123 : 456", ResultMap( + GoCompiler -> + """var tmp1 int8; + |if (2 < 3) { + | tmp1 = 123 + |} else { + | tmp1 = 456 + |} + |tmp1""".stripMargin, + LuaCompiler -> "utils.box_unwrap((2 < 3) and utils.box_wrap(123) or (456))", + PythonCompiler -> "123 if 2 < 3 else 456", + )) + + // When evaluated, must be 12 + 34 = 46 + everybodyExcept("2 < 3 ? 12 + 34 : 45 + 67", "2 < 3 ? 12 + 34 : 45 + 67", ResultMap( + GoCompiler -> + """var tmp1 int; + |if (2 < 3) { + | tmp1 = 12 + 34 + |} else { + | tmp1 = 45 + 67 + |} + |tmp1""".stripMargin, + LuaCompiler -> "utils.box_unwrap((2 < 3) and utils.box_wrap(12 + 34) or (45 + 67))", + PythonCompiler -> "12 + 34 if 2 < 3 else 45 + 67", + )) + + // When evaluated, must be (12 + 34) + 67 = 113 + everybodyExcept("(2 < 3 ? 12 + 34 : 45) + 67", "(2 < 3 ? 12 + 34 : 45) + 67", ResultMap( + GoCompiler -> + """var tmp1 int; + |if (2 < 3) { + | tmp1 = 12 + 34 + |} else { + | tmp1 = 45 + |} + |tmp1 + 67""".stripMargin, + LuaCompiler -> "utils.box_unwrap((2 < 3) and utils.box_wrap(12 + 34) or (45)) + 67", + PythonCompiler -> "(12 + 34 if 2 < 3 else 45) + 67", + )) + full("2 < 3 ? \"foo\" : \"bar\"", CalcIntType, CalcStrType, ResultMap( - CppCompiler -> "((2 < 3) ? (std::string(\"foo\")) : (std::string(\"bar\")))", - CSharpCompiler -> "(2 < 3 ? \"foo\" : \"bar\")", + CppCompiler -> "2 < 3 ? std::string(\"foo\") : std::string(\"bar\")", + CSharpCompiler -> "2 < 3 ? \"foo\" : \"bar\"", GoCompiler -> """var tmp1 string; |if (2 < 3) { @@ -237,13 +278,53 @@ class TranslatorSpec extends AnyFunSpec { | tmp1 = "bar" |} |tmp1""".stripMargin, - JavaCompiler -> "(2 < 3 ? \"foo\" : \"bar\")", - JavaScriptCompiler -> "(2 < 3 ? \"foo\" : \"bar\")", + JavaCompiler -> "2 < 3 ? \"foo\" : \"bar\"", + JavaScriptCompiler -> "2 < 3 ? \"foo\" : \"bar\"", LuaCompiler -> "utils.box_unwrap((2 < 3) and utils.box_wrap(\"foo\") or (\"bar\"))", - PerlCompiler -> "(2 < 3 ? \"foo\" : \"bar\")", - PHPCompiler -> "(2 < 3 ? \"foo\" : \"bar\")", - PythonCompiler -> "(u\"foo\" if 2 < 3 else u\"bar\")", - RubyCompiler -> "(2 < 3 ? \"foo\" : \"bar\")" + PerlCompiler -> "2 < 3 ? \"foo\" : \"bar\"", + PHPCompiler -> "2 < 3 ? \"foo\" : \"bar\"", + PythonCompiler -> "u\"foo\" if 2 < 3 else u\"bar\"", + RubyCompiler -> "2 < 3 ? \"foo\" : \"bar\"" + )) + + full("true ? foo._io : bar._io", KaitaiStructType, KaitaiStreamType, ResultMap( + CppCompiler -> "true ? foo()->_io() : bar()->_io()", + CSharpCompiler -> "true ? Foo.M_Io : Bar.M_Io", + GoCompiler -> + """var tmp1 *kaitai.Stream; + |if (true) { + | tmp1 = this.Foo._io + |} else { + | tmp1 = this.Bar._io + |} + |tmp1""".stripMargin, + JavaCompiler -> "true ? foo()._io() : bar()._io()", + JavaScriptCompiler -> "true ? this.foo._io : this.bar._io", + LuaCompiler -> "utils.box_unwrap((true) and utils.box_wrap(self.foo._io) or (self.bar._io))", + PerlCompiler -> "1 ? $self->foo()->_io() : $self->bar()->_io()", + PHPCompiler -> "true ? $this->foo()->_io() : $this->bar()->_io()", + PythonCompiler -> "self.foo._io if True else self.bar._io", + RubyCompiler -> "true ? foo._io : bar._io", + )) + + full("(true ? foo : bar)._io", KaitaiStructType, KaitaiStreamType, ResultMap( + CppCompiler -> "(true ? foo() : bar())->_io()", + CSharpCompiler -> "(true ? Foo : Bar).M_Io", + GoCompiler -> + """var tmp1 interface{}; + |if (true) { + | tmp1 = this.Foo + |} else { + | tmp1 = this.Bar + |} + |tmp1._io""".stripMargin, + JavaCompiler -> "(true ? foo() : bar())._io()", + JavaScriptCompiler -> "(true ? this.foo : this.bar)._io", + LuaCompiler -> "utils.box_unwrap((true) and utils.box_wrap(self.foo) or (self.bar))._io", + PerlCompiler -> "(1 ? $self->foo() : $self->bar())->_io()", + PHPCompiler -> "(true ? $this->foo() : $this->bar())->_io()", + PythonCompiler -> "(self.foo if True else self.bar)._io", + RubyCompiler -> "(true ? foo : bar)._io", )) } diff --git a/shared/src/main/scala/io/kaitai/struct/translators/BaseTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/BaseTranslator.scala index f2b3a32e0..83e713152 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/BaseTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/BaseTranslator.scala @@ -122,7 +122,7 @@ abstract class BaseTranslator(val provider: TypeProvider) case Ast.expr.BoolOp(op: Ast.boolop, values: Seq[Ast.expr]) => doBooleanOp(op, values) case Ast.expr.IfExp(condition, ifTrue, ifFalse) => - doIfExp(condition, ifTrue, ifFalse) + doIfExp(condition, ifTrue, ifFalse, extPrec) case Ast.expr.Subscript(container: Ast.expr, idx: Ast.expr) => detectType(idx) match { case _: IntType => @@ -152,7 +152,6 @@ abstract class BaseTranslator(val provider: TypeProvider) } } - def doIfExp(condition: Ast.expr, ifTrue: Ast.expr, ifFalse: Ast.expr): String def doCast(value: Ast.expr, typeName: DataType): String = translate(value) def doByteSizeOfType(typeName: Ast.typeId): String = doIntLiteral( CommonSizeOf.bitToByteSize( @@ -192,8 +191,10 @@ abstract class BaseTranslator(val provider: TypeProvider) // Predefined methods of various types def strConcat(left: Ast.expr, right: Ast.expr, extPrec: Int) = genericBinOp(left, Ast.operator.Add, right, extPrec) - def boolToInt(value: Ast.expr): String = - doIfExp(value, Ast.expr.IntNum(1), Ast.expr.IntNum(0)) + def boolToInt(value: Ast.expr): String = { + // TODO: fix METHOD_PRECEDENCE with proper propagation + doIfExp(value, Ast.expr.IntNum(1), Ast.expr.IntNum(0), METHOD_PRECEDENCE) + } def kaitaiStreamSize(value: Ast.expr): String = anyField(value, "size") def kaitaiStreamEof(value: Ast.expr): String = anyField(value, "is_eof") diff --git a/shared/src/main/scala/io/kaitai/struct/translators/CSharpTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/CSharpTranslator.scala index dcc8492ab..eeb1568ab 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/CSharpTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/CSharpTranslator.scala @@ -86,8 +86,6 @@ class CSharpTranslator(provider: TypeProvider, importList: ImportList) extends B override def arraySubscript(container: expr, idx: expr): String = s"${translate(container, METHOD_PRECEDENCE)}[${translate(idx)}]" - override def doIfExp(condition: expr, ifTrue: expr, ifFalse: expr): String = - s"(${translate(condition)} ? ${translate(ifTrue)} : ${translate(ifFalse)})" override def doCast(value: Ast.expr, typeName: DataType): String = s"((${CSharpCompiler.kaitaiType2NativeType(typeName)}) (${translate(value)}))" diff --git a/shared/src/main/scala/io/kaitai/struct/translators/CommonOps.scala b/shared/src/main/scala/io/kaitai/struct/translators/CommonOps.scala index 3d1482d0c..46b664a30 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/CommonOps.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/CommonOps.scala @@ -26,6 +26,8 @@ trait CommonOps extends AbstractTranslator { Ast.operator.BitOr -> 80, ) + val IF_EXP_PRECEDENCE = 30 + def genericBinOp(left: Ast.expr, op: Ast.operator, right: Ast.expr, extPrec: Int): String = genericBinOpStr(left, op, binOp(op), right, extPrec) @@ -97,4 +99,16 @@ trait CommonOps extends AbstractTranslator { case Ast.unaryop.Minus => "-" case Ast.unaryop.Not => "!" } + + def doIfExp(condition: Ast.expr, ifTrue: Ast.expr, ifFalse: Ast.expr, extPrec: Int): String = { + val conditionStr = translate(condition, IF_EXP_PRECEDENCE) + val ifTrueStr = translate(ifTrue, IF_EXP_PRECEDENCE) + val ifFalseStr = translate(ifFalse, IF_EXP_PRECEDENCE) + val fullStr = s"$conditionStr ? $ifTrueStr : $ifFalseStr" + if (IF_EXP_PRECEDENCE <= extPrec) { + s"($fullStr)" + } else { + fullStr + } + } } diff --git a/shared/src/main/scala/io/kaitai/struct/translators/CppTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/CppTranslator.scala index b5686af2a..0d1543d67 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/CppTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/CppTranslator.scala @@ -128,7 +128,7 @@ class CppTranslator(provider: TypeProvider, importListSrc: CppImportList, import } override def anyField(value: expr, attrName: String): String = - s"${translate(value)}->${doName(attrName)}" + s"${translate(value, METHOD_PRECEDENCE)}->${doName(attrName)}" override def doName(s: String) = s match { case Identifier.ITERATOR => "_" @@ -158,8 +158,6 @@ class CppTranslator(provider: TypeProvider, importListSrc: CppImportList, import override def arraySubscript(container: expr, idx: expr): String = s"${translate(container)}->at(${translate(idx)})" - override def doIfExp(condition: expr, ifTrue: expr, ifFalse: expr): String = - s"((${translate(condition)}) ? (${translate(ifTrue)}) : (${translate(ifFalse)}))" override def doCast(value: Ast.expr, typeName: DataType): String = config.cppConfig.pointers match { case RawPointers | UniqueAndRawPointers => diff --git a/shared/src/main/scala/io/kaitai/struct/translators/JavaScriptTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/JavaScriptTranslator.scala index 9f0cafbab..0817cc744 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/JavaScriptTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/JavaScriptTranslator.scala @@ -67,8 +67,6 @@ class JavaScriptTranslator(provider: TypeProvider) extends BaseTranslator(provid override def arraySubscript(container: expr, idx: expr): String = s"${translate(container)}[${translate(idx)}]" - override def doIfExp(condition: expr, ifTrue: expr, ifFalse: expr): String = - s"(${translate(condition)} ? ${translate(ifTrue)} : ${translate(ifFalse)})" // Predefined methods of various types override def strToInt(s: expr, base: expr): String = diff --git a/shared/src/main/scala/io/kaitai/struct/translators/JavaTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/JavaTranslator.scala index aa584b0ce..65ae017eb 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/JavaTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/JavaTranslator.scala @@ -100,8 +100,6 @@ class JavaTranslator(provider: TypeProvider, importList: ImportList) extends Bas override def arraySubscript(container: expr, idx: expr): String = s"${translate(container)}.get((int) ${translate(idx, METHOD_PRECEDENCE)})" - override def doIfExp(condition: expr, ifTrue: expr, ifFalse: expr): String = - s"(${translate(condition)} ? ${translate(ifTrue)} : ${translate(ifFalse)})" override def doCast(value: Ast.expr, typeName: DataType): String = s"((${JavaCompiler.kaitaiType2JavaType(typeName)}) (${translate(value)}))" diff --git a/shared/src/main/scala/io/kaitai/struct/translators/LuaTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/LuaTranslator.scala index c9ef6ce0f..f2b30b91a 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/LuaTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/LuaTranslator.scala @@ -58,7 +58,7 @@ class LuaTranslator(provider: TypeProvider, importList: ImportList) extends Base // Lua indexes start at 1, so we need to offset them s"${translate(container)}[${translate(idx)} + 1]" } - override def doIfExp(condition: Ast.expr, ifTrue: Ast.expr, ifFalse: Ast.expr): String = { + override def doIfExp(condition: Ast.expr, ifTrue: Ast.expr, ifFalse: Ast.expr, extPrec: Int): String = { importList.add("local utils = require(\"utils\")") // http://lua-users.org/wiki/TernaryOperator (section Boxing/unboxing, using functions) diff --git a/shared/src/main/scala/io/kaitai/struct/translators/NimTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/NimTranslator.scala index cd1cd107e..883259eb8 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/NimTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/NimTranslator.scala @@ -37,7 +37,7 @@ class NimTranslator(provider: TypeProvider, importList: ImportList) extends Base case Identifier.ROOT => s"${ksToNim(provider.determineType(Identifier.ROOT))}(this.${doName(s)})" case _ => s"this.${doName(s)}" } - override def doIfExp(condition: expr, ifTrue: expr, ifFalse: expr): String = + override def doIfExp(condition: expr, ifTrue: expr, ifFalse: expr, extPrec: Int): String = s"(if ${translate(condition)}: ${translate(ifTrue)} else: ${translate(ifFalse)})" override def arraySubscript(container: expr, idx: expr): String = s"${translate(container)}[${translate(idx)}]" diff --git a/shared/src/main/scala/io/kaitai/struct/translators/PHPTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/PHPTranslator.scala index cf46da9e9..503f99da0 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/PHPTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/PHPTranslator.scala @@ -54,7 +54,7 @@ class PHPTranslator(provider: TypeProvider, config: RuntimeConfig) extends BaseT } override def anyField(value: expr, attrName: String): String = - s"${translate(value)}->${doName(attrName)}" + s"${translate(value, METHOD_PRECEDENCE)}->${doName(attrName)}" override def doLocalName(s: String) = { s match { @@ -80,8 +80,6 @@ class PHPTranslator(provider: TypeProvider, config: RuntimeConfig) extends BaseT override def arraySubscript(container: expr, idx: expr): String = s"${translate(container)}[${translate(idx)}]" - override def doIfExp(condition: expr, ifTrue: expr, ifFalse: expr): String = - s"(${translate(condition)} ? ${translate(ifTrue)} : ${translate(ifFalse)})" // Predefined methods of various types override def strConcat(left: expr, right: expr, extPrec: Int) = diff --git a/shared/src/main/scala/io/kaitai/struct/translators/PerlTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/PerlTranslator.scala index fbf0b53ac..f88bac688 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/PerlTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/PerlTranslator.scala @@ -54,7 +54,7 @@ class PerlTranslator(provider: TypeProvider, importList: ImportList) extends Bas s"pack('C*', (${elts.map(translate).mkString(", ")}))" override def anyField(value: Ast.expr, attrName: String): String = - s"${translate(value)}->${doName(attrName)}" + s"${translate(value, METHOD_PRECEDENCE)}->${doName(attrName)}" override def doLocalName(s: String) = { s match { @@ -108,8 +108,6 @@ class PerlTranslator(provider: TypeProvider, importList: ImportList) extends Bas override def arraySubscript(container: Ast.expr, idx: Ast.expr): String = s"@{${translate(container)}}[${translate(idx)}]" - override def doIfExp(condition: Ast.expr, ifTrue: Ast.expr, ifFalse: Ast.expr): String = - s"(${translate(condition)} ? ${translate(ifTrue)} : ${translate(ifFalse)})" // Predefined methods of various types override def strConcat(left: Ast.expr, right: Ast.expr, extPrec: Int) = diff --git a/shared/src/main/scala/io/kaitai/struct/translators/PythonTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/PythonTranslator.scala index 2764e0bb0..7f7411893 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/PythonTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/PythonTranslator.scala @@ -73,8 +73,17 @@ class PythonTranslator(provider: TypeProvider, importList: ImportList) extends B override def arraySubscript(container: Ast.expr, idx: Ast.expr): String = s"${translate(container)}[${translate(idx)}]" - override def doIfExp(condition: Ast.expr, ifTrue: Ast.expr, ifFalse: Ast.expr): String = - s"(${translate(ifTrue)} if ${translate(condition)} else ${translate(ifFalse)})" + override def doIfExp(condition: Ast.expr, ifTrue: Ast.expr, ifFalse: Ast.expr, extPrec: Int): String = { + val conditionStr = translate(condition, IF_EXP_PRECEDENCE) + val ifTrueStr = translate(ifTrue, IF_EXP_PRECEDENCE) + val ifFalseStr = translate(ifFalse, IF_EXP_PRECEDENCE) + val fullStr = s"$ifTrueStr if $conditionStr else $ifFalseStr" + if (IF_EXP_PRECEDENCE <= extPrec) { + s"($fullStr)" + } else { + fullStr + } + } // Predefined methods of various types override def strToInt(s: Ast.expr, base: Ast.expr): String = { diff --git a/shared/src/main/scala/io/kaitai/struct/translators/RubyTranslator.scala b/shared/src/main/scala/io/kaitai/struct/translators/RubyTranslator.scala index f08e6a1f5..fc9fd8d1b 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/RubyTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/RubyTranslator.scala @@ -69,8 +69,6 @@ class RubyTranslator(provider: TypeProvider) extends BaseTranslator(provider) override def arraySubscript(container: Ast.expr, idx: Ast.expr): String = s"${translate(container, METHOD_PRECEDENCE)}[${translate(idx)}]" - override def doIfExp(condition: Ast.expr, ifTrue: Ast.expr, ifFalse: Ast.expr): String = - s"(${translate(condition)} ? ${translate(ifTrue)} : ${translate(ifFalse)})" // Predefined methods of various types override def strToInt(s: Ast.expr, base: 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 index d72ac5573..4a2b9b3c2 100644 --- a/shared/src/main/scala/io/kaitai/struct/translators/RustTranslator.scala +++ b/shared/src/main/scala/io/kaitai/struct/translators/RustTranslator.scala @@ -47,7 +47,7 @@ class RustTranslator(provider: TypeProvider, config: RuntimeConfig) extends Base override def arraySubscript(container: expr, idx: expr): String = s"${translate(container)}[${translate(idx)}]" - override def doIfExp(condition: expr, ifTrue: expr, ifFalse: expr): String = + override def doIfExp(condition: expr, ifTrue: expr, ifFalse: expr, extPrec: Int): String = "if " + translate(condition) + " { " + translate(ifTrue) + " } else { " + translate(ifFalse) + "}"