From 10ddd5b127d8d41316b0f8b552dfcb0a0f91a870 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 12 Jul 2022 21:58:33 +0200 Subject: [PATCH] fixed missing non-boolean operand cast in logical expressions --- .../compiler/astprocessing/AstChecker.kt | 2 +- .../compiler/astprocessing/BoolRemover.kt | 61 ++++---- .../compiler/astprocessing/TypecastsAdder.kt | 136 ++++++++++-------- docs/source/todo.rst | 3 - examples/test.p8 | 18 ++- 5 files changed, 124 insertions(+), 96 deletions(-) diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 8520a9947..bf870ff53 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -913,7 +913,7 @@ internal class AstChecker(private val program: Program, if(expr.operator !in ComparisonOperators) { if (leftDt == DataType.STR && rightDt == DataType.STR || leftDt in ArrayDatatypes && rightDt in ArrayDatatypes) { // str+str and str*number have already been const evaluated before we get here. - errors.err("no computational expressions with strings or arrays are possible", expr.position) + errors.err("no computational or logical expressions with strings or arrays are possible", expr.position) } } diff --git a/compiler/src/prog8/compiler/astprocessing/BoolRemover.kt b/compiler/src/prog8/compiler/astprocessing/BoolRemover.kt index e7f132c82..30df7e7b5 100644 --- a/compiler/src/prog8/compiler/astprocessing/BoolRemover.kt +++ b/compiler/src/prog8/compiler/astprocessing/BoolRemover.kt @@ -82,35 +82,40 @@ internal class BoolRemover(val program: Program) : AstWalker() { "xor" -> "^" else -> "invalid" } - return listOf( - IAstModification.ReplaceNodeSafe(expr.left, wrapWithBooleanCastIfNeeded(expr.left), expr), - IAstModification.ReplaceNodeSafe(expr.right, wrapWithBooleanCastIfNeeded(expr.right), expr),) + val mods = mutableListOf() + val newLeft = wrapWithBooleanCastIfNeeded(expr.left, program) + val newRight = wrapWithBooleanCastIfNeeded(expr.right, program) + if(newLeft!=null) + mods += IAstModification.ReplaceNodeSafe(expr.left, newLeft, expr) + if(newRight!=null) + mods += IAstModification.ReplaceNodeSafe(expr.right, newRight, expr) + return mods } return noModifications } - - private fun wrapWithBooleanCastIfNeeded(expr: Expression): Expression { - fun isBoolean(expr: Expression): Boolean { - return if(expr.inferType(program) istype DataType.BOOL) - true - else if(expr is NumericLiteral && expr.type in IntegerDatatypes && (expr.number==0.0 || expr.number==1.0)) - true - else if(expr is BinaryExpression && expr.operator in ComparisonOperators + LogicalOperators) - true - else if(expr is PrefixExpression && expr.operator == "not") - true - else if(expr is BinaryExpression && expr.operator in BitwiseOperators) { - if(isBoolean(expr.left) && isBoolean(expr.right)) - true - else expr.operator=="&" && expr.right.constValue(program)?.number==1.0 // x & 1 is also a boolean result - } - else - false - } - - return if(isBoolean(expr)) - expr - else - TypecastExpression(expr, DataType.BOOL, true, expr.position) - } +} + +internal fun wrapWithBooleanCastIfNeeded(expr: Expression, program: Program): Expression? { + fun isBoolean(expr: Expression): Boolean { + return if(expr.inferType(program) istype DataType.BOOL) + true + else if(expr is NumericLiteral && expr.type in IntegerDatatypes && (expr.number==0.0 || expr.number==1.0)) + true + else if(expr is BinaryExpression && expr.operator in ComparisonOperators + LogicalOperators) + true + else if(expr is PrefixExpression && expr.operator == "not") + true + else if(expr is BinaryExpression && expr.operator in BitwiseOperators) { + if(isBoolean(expr.left) && isBoolean(expr.right)) + true + else expr.operator=="&" && expr.right.constValue(program)?.number==1.0 // x & 1 is also a boolean result + } + else + false + } + + return if(isBoolean(expr)) + null + else + TypecastExpression(expr, DataType.BOOL, true, expr.position) } diff --git a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt index 03654479a..e3a7749e6 100644 --- a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt +++ b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt @@ -46,71 +46,85 @@ class TypecastsAdder(val program: Program, val options: CompilationOptions, val val leftCv = expr.left.constValue(program) val rightCv = expr.right.constValue(program) - if(leftDt.isKnown && rightDt.isKnown && leftDt!=rightDt) { - - // convert bool type to byte if needed - if(leftDt istype DataType.BOOL && rightDt.isBytes && !rightDt.istype(DataType.BOOL)) { - if(rightCv==null || (rightCv.number!=1.0 && rightCv.number!=0.0)) - return listOf(IAstModification.ReplaceNode(expr.left, - TypecastExpression(expr.left, rightDt.getOr(DataType.UNDEFINED),true, expr.left.position), expr)) - } else if(leftDt.isBytes && !leftDt.istype(DataType.BOOL) && rightDt istype DataType.BOOL) { - if(leftCv==null || (leftCv.number!=1.0 && leftCv.number!=0.0)) - return listOf(IAstModification.ReplaceNode(expr.right, - TypecastExpression(expr.right, leftDt.getOr(DataType.UNDEFINED),true, expr.right.position), expr)) - } - - // convert a negative operand for bitwise operator to the 2's complement positive number instead - if(expr.operator in BitwiseOperators && leftDt.isInteger && rightDt.isInteger) { - if(leftCv!=null && leftCv.number<0) { - val value = if(rightDt.isBytes) 256+leftCv.number else 65536+leftCv.number - return listOf(IAstModification.ReplaceNode( - expr.left, - NumericLiteral(rightDt.getOr(DataType.UNDEFINED), value, expr.left.position), - expr)) - } - if(rightCv!=null && rightCv.number<0) { - val value = if(leftDt.isBytes) 256+rightCv.number else 65536+rightCv.number - return listOf(IAstModification.ReplaceNode( - expr.right, - NumericLiteral(leftDt.getOr(DataType.UNDEFINED), value, expr.right.position), - expr)) - } - - if(leftDt istype DataType.BYTE && rightDt.oneOf(DataType.UBYTE, DataType.UWORD)) { - // cast left to unsigned - val cast = TypecastExpression(expr.left, rightDt.getOr(DataType.UNDEFINED), true, expr.left.position) - return listOf(IAstModification.ReplaceNode(expr.left, cast, expr)) - } - if(leftDt istype DataType.WORD && rightDt.oneOf(DataType.UBYTE, DataType.UWORD)) { - // cast left to unsigned - val cast = TypecastExpression(expr.left, rightDt.getOr(DataType.UNDEFINED), true, expr.left.position) - return listOf(IAstModification.ReplaceNode(expr.left, cast, expr)) - } - if(rightDt istype DataType.BYTE && leftDt.oneOf(DataType.UBYTE, DataType.UWORD)) { - // cast right to unsigned - val cast = TypecastExpression(expr.right, leftDt.getOr(DataType.UNDEFINED), true, expr.right.position) - return listOf(IAstModification.ReplaceNode(expr.right, cast, expr)) - } - if(rightDt istype DataType.WORD && leftDt.oneOf(DataType.UBYTE, DataType.UWORD)) { - // cast right to unsigned - val cast = TypecastExpression(expr.right, leftDt.getOr(DataType.UNDEFINED), true, expr.right.position) - return listOf(IAstModification.ReplaceNode(expr.right, cast, expr)) - } - } - - - // determine common datatype and add typecast as required to make left and right equal types - val (commonDt, toFix) = BinaryExpression.commonDatatype(leftDt.getOr(DataType.UNDEFINED), rightDt.getOr(DataType.UNDEFINED), expr.left, expr.right) - if(toFix!=null) { + if(leftDt.isKnown && rightDt.isKnown) { + if(expr.operator in LogicalOperators && leftDt.isInteger && rightDt.isInteger) { + // see if any of the operands needs conversion to bool val modifications = mutableListOf() - when { - toFix===expr.left -> addTypecastOrCastedValueModification(modifications, expr.left, commonDt, expr) - toFix===expr.right -> addTypecastOrCastedValueModification(modifications, expr.right, commonDt, expr) - else -> throw FatalAstException("confused binary expression side") + val newLeft = wrapWithBooleanCastIfNeeded(expr.left, program) + val newRight = wrapWithBooleanCastIfNeeded(expr.right, program) + if(newLeft!=null) + modifications += IAstModification.ReplaceNode(expr.left, newLeft, expr) + if(newRight!=null) + modifications += IAstModification.ReplaceNode(expr.right, newRight, expr) + if(modifications.isNotEmpty()) + return modifications + } + if(leftDt!=rightDt) { + // convert bool type to byte if needed + if(leftDt istype DataType.BOOL && rightDt.isBytes && !rightDt.istype(DataType.BOOL)) { + if(rightCv==null || (rightCv.number!=1.0 && rightCv.number!=0.0)) + return listOf(IAstModification.ReplaceNode(expr.left, + TypecastExpression(expr.left, rightDt.getOr(DataType.UNDEFINED),true, expr.left.position), expr)) + } else if(leftDt.isBytes && !leftDt.istype(DataType.BOOL) && rightDt istype DataType.BOOL) { + if(leftCv==null || (leftCv.number!=1.0 && leftCv.number!=0.0)) + return listOf(IAstModification.ReplaceNode(expr.right, + TypecastExpression(expr.right, leftDt.getOr(DataType.UNDEFINED),true, expr.right.position), expr)) + } + + // convert a negative operand for bitwise operator to the 2's complement positive number instead + if(expr.operator in BitwiseOperators && leftDt.isInteger && rightDt.isInteger) { + if(leftCv!=null && leftCv.number<0) { + val value = if(rightDt.isBytes) 256+leftCv.number else 65536+leftCv.number + return listOf(IAstModification.ReplaceNode( + expr.left, + NumericLiteral(rightDt.getOr(DataType.UNDEFINED), value, expr.left.position), + expr)) + } + if(rightCv!=null && rightCv.number<0) { + val value = if(leftDt.isBytes) 256+rightCv.number else 65536+rightCv.number + return listOf(IAstModification.ReplaceNode( + expr.right, + NumericLiteral(leftDt.getOr(DataType.UNDEFINED), value, expr.right.position), + expr)) + } + + if(leftDt istype DataType.BYTE && rightDt.oneOf(DataType.UBYTE, DataType.UWORD)) { + // cast left to unsigned + val cast = TypecastExpression(expr.left, rightDt.getOr(DataType.UNDEFINED), true, expr.left.position) + return listOf(IAstModification.ReplaceNode(expr.left, cast, expr)) + } + if(leftDt istype DataType.WORD && rightDt.oneOf(DataType.UBYTE, DataType.UWORD)) { + // cast left to unsigned + val cast = TypecastExpression(expr.left, rightDt.getOr(DataType.UNDEFINED), true, expr.left.position) + return listOf(IAstModification.ReplaceNode(expr.left, cast, expr)) + } + if(rightDt istype DataType.BYTE && leftDt.oneOf(DataType.UBYTE, DataType.UWORD)) { + // cast right to unsigned + val cast = TypecastExpression(expr.right, leftDt.getOr(DataType.UNDEFINED), true, expr.right.position) + return listOf(IAstModification.ReplaceNode(expr.right, cast, expr)) + } + if(rightDt istype DataType.WORD && leftDt.oneOf(DataType.UBYTE, DataType.UWORD)) { + // cast right to unsigned + val cast = TypecastExpression(expr.right, leftDt.getOr(DataType.UNDEFINED), true, expr.right.position) + return listOf(IAstModification.ReplaceNode(expr.right, cast, expr)) + } + } + + + // determine common datatype and add typecast as required to make left and right equal types + val (commonDt, toFix) = BinaryExpression.commonDatatype(leftDt.getOr(DataType.UNDEFINED), rightDt.getOr(DataType.UNDEFINED), expr.left, expr.right) + if(toFix!=null) { + val modifications = mutableListOf() + when { + toFix===expr.left -> addTypecastOrCastedValueModification(modifications, expr.left, commonDt, expr) + toFix===expr.right -> addTypecastOrCastedValueModification(modifications, expr.right, commonDt, expr) + else -> throw FatalAstException("confused binary expression side") + } + return modifications } - return modifications } } + return noModifications } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 83c8d6634..692aa7b4b 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,9 +3,6 @@ TODO For next release ^^^^^^^^^^^^^^^^ -- fix compiler bug: tehtriz: blocks fall through the bottom -- fix compiler bug: maze: maze runner goes all wacky (both c64 and cx6) - ... diff --git a/examples/test.p8 b/examples/test.p8 index 076b48991..df41c534e 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -3,8 +3,20 @@ main { sub start() { - bool bb2=true - bool @shared bb = bb2 and true - txt.print_ub(bb) + ubyte value1 = %1110 + ubyte value2 = %1111 + + bool[2] @shared barr = [true, false] + +; if value1 and value2 ; TODO value1 isn't converted to bool in 6502 preprocess +; txt.print("ok") +; else +; txt.print("fail!") +; txt.nl() + + if value1 and value2!=255 ; TODO value1 isn't converted to bool in 6502 preprocess + txt.print("ok") + else + txt.print("fail!") } }