From 9a3dab20dc5ed8536d77fba1f22396a89eb323c4 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Wed, 10 Jul 2019 08:30:17 +0200 Subject: [PATCH] extra warnings about register usage in loops --- compiler/src/prog8/ast/Interfaces.kt | 2 +- .../prog8/ast/expressions/AstExpressions.kt | 23 ++++++----- .../src/prog8/ast/processing/AstChecker.kt | 17 ++++++-- .../src/prog8/optimizer/ConstantFolding.kt | 2 +- examples/test.p8 | 39 ++++++------------- 5 files changed, 38 insertions(+), 45 deletions(-) diff --git a/compiler/src/prog8/ast/Interfaces.kt b/compiler/src/prog8/ast/Interfaces.kt index 51d7967ac..43f01c183 100644 --- a/compiler/src/prog8/ast/Interfaces.kt +++ b/compiler/src/prog8/ast/Interfaces.kt @@ -173,7 +173,7 @@ interface IExpression: Node { fun constValue(program: Program): LiteralValue? fun accept(visitor: IAstModifyingVisitor): IExpression fun accept(visitor: IAstVisitor) - fun referencesIdentifier(name: String): Boolean + fun referencesIdentifiers(vararg name: String): Boolean fun inferType(program: Program): DataType? infix fun isSameAs(other: IExpression): Boolean { diff --git a/compiler/src/prog8/ast/expressions/AstExpressions.kt b/compiler/src/prog8/ast/expressions/AstExpressions.kt index 504a156d5..dd77e6fc9 100644 --- a/compiler/src/prog8/ast/expressions/AstExpressions.kt +++ b/compiler/src/prog8/ast/expressions/AstExpressions.kt @@ -13,7 +13,6 @@ import prog8.functions.BuiltinFunctions import prog8.functions.NotConstArgumentException import prog8.functions.builtinFunctionReturnType import kotlin.math.abs -import kotlin.math.floor val associativeOperators = setOf("+", "*", "&", "|", "^", "or", "and", "xor", "==", "!=") @@ -30,7 +29,7 @@ class PrefixExpression(val operator: String, var expression: IExpression, overri override fun constValue(program: Program): LiteralValue? = null override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) - override fun referencesIdentifier(name: String) = expression.referencesIdentifier(name) + override fun referencesIdentifiers(vararg name: String) = expression.referencesIdentifiers(*name) override fun inferType(program: Program): DataType? = expression.inferType(program) override fun toString(): String { @@ -56,7 +55,7 @@ class BinaryExpression(var left: IExpression, var operator: String, var right: I override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) - override fun referencesIdentifier(name: String) = left.referencesIdentifier(name) || right.referencesIdentifier(name) + override fun referencesIdentifiers(vararg name: String) = left.referencesIdentifiers(*name) || right.referencesIdentifiers(*name) override fun inferType(program: Program): DataType? { val leftDt = left.inferType(program) val rightDt = right.inferType(program) @@ -224,7 +223,7 @@ class ArrayIndexedExpression(val identifier: IdentifierReference, override fun constValue(program: Program): LiteralValue? = null override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) - override fun referencesIdentifier(name: String) = identifier.referencesIdentifier(name) + override fun referencesIdentifiers(vararg name: String) = identifier.referencesIdentifiers(*name) override fun inferType(program: Program): DataType? { val target = identifier.targetStatement(program.namespace) @@ -258,7 +257,7 @@ class TypecastExpression(var expression: IExpression, var type: DataType, val im override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) - override fun referencesIdentifier(name: String) = expression.referencesIdentifier(name) + override fun referencesIdentifiers(vararg name: String) = expression.referencesIdentifiers(*name) override fun inferType(program: Program): DataType? = type override fun constValue(program: Program): LiteralValue? { val cv = expression.constValue(program) ?: return null @@ -282,7 +281,7 @@ data class AddressOf(val identifier: IdentifierReference, override val position: var scopedname: String? = null // will be set in a later state by the compiler override fun constValue(program: Program): LiteralValue? = null - override fun referencesIdentifier(name: String) = false + override fun referencesIdentifiers(vararg name: String) = false override fun inferType(program: Program) = DataType.UWORD override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) @@ -298,7 +297,7 @@ class DirectMemoryRead(var addressExpression: IExpression, override val position override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) - override fun referencesIdentifier(name: String) = false + override fun referencesIdentifiers(vararg name: String) = false override fun inferType(program: Program): DataType? = DataType.UBYTE override fun constValue(program: Program): LiteralValue? = null @@ -317,7 +316,7 @@ open class LiteralValue(val type: DataType, override val position: Position) : IExpression { override lateinit var parent: Node - override fun referencesIdentifier(name: String) = arrayvalue?.any { it.referencesIdentifier(name) } ?: false + override fun referencesIdentifiers(vararg name: String) = arrayvalue?.any { it.referencesIdentifiers(*name) } ?: false val isString = type in StringDatatypes val isNumeric = type in NumericDatatypes @@ -584,7 +583,7 @@ class RangeExpr(var from: IExpression, override fun constValue(program: Program): LiteralValue? = null override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) - override fun referencesIdentifier(name: String): Boolean = from.referencesIdentifier(name) || to.referencesIdentifier(name) + override fun referencesIdentifiers(vararg name: String): Boolean = from.referencesIdentifiers(*name) || to.referencesIdentifiers(*name) override fun inferType(program: Program): DataType? { val fromDt=from.inferType(program) val toDt=to.inferType(program) @@ -653,7 +652,7 @@ class RegisterExpr(val register: Register, override val position: Position) : IE override fun constValue(program: Program): LiteralValue? = null override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) - override fun referencesIdentifier(name: String): Boolean = false + override fun referencesIdentifiers(vararg name: String): Boolean = register.name in name override fun toString(): String { return "RegisterExpr(register=$register, pos=$position)" } @@ -695,7 +694,7 @@ data class IdentifierReference(val nameInSource: List, override val posi override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) - override fun referencesIdentifier(name: String): Boolean = nameInSource.last() == name // @todo is this correct all the time? + override fun referencesIdentifiers(vararg name: String): Boolean = nameInSource.last() in name // @todo is this correct all the time? override fun inferType(program: Program): DataType? { val targetStmt = targetStatement(program.namespace) @@ -761,7 +760,7 @@ class FunctionCall(override var target: IdentifierReference, override fun accept(visitor: IAstModifyingVisitor) = visitor.visit(this) override fun accept(visitor: IAstVisitor) = visitor.visit(this) - override fun referencesIdentifier(name: String): Boolean = target.referencesIdentifier(name) || arglist.any{it.referencesIdentifier(name)} + override fun referencesIdentifiers(vararg name: String): Boolean = target.referencesIdentifiers(*name) || arglist.any{it.referencesIdentifiers(*name)} override fun inferType(program: Program): DataType? { val constVal = constValue(program ,false) diff --git a/compiler/src/prog8/ast/processing/AstChecker.kt b/compiler/src/prog8/ast/processing/AstChecker.kt index 2b132976a..5e37d00ed 100644 --- a/compiler/src/prog8/ast/processing/AstChecker.kt +++ b/compiler/src/prog8/ast/processing/AstChecker.kt @@ -117,7 +117,7 @@ internal class AstChecker(private val program: Program, checkResult.add(ExpressionError("can only loop over an iterable type", forLoop.position)) } else { if (forLoop.loopRegister != null) { - printWarning("using a register as loop variable is risky (it could get clobbered in the body)", forLoop.position) + printWarning("using a register as loop variable is risky (it could get clobbered)", forLoop.position) // loop register if (iterableDt != DataType.UBYTE && iterableDt!= DataType.ARRAY_UB && iterableDt !in StringDatatypes) checkResult.add(ExpressionError("register can only loop over bytes", forLoop.position)) @@ -126,7 +126,6 @@ internal class AstChecker(private val program: Program, val loopvar = forLoop.loopVar!!.targetVarDecl(program.namespace) if(loopvar==null || loopvar.type== VarDeclType.CONST) { checkResult.add(SyntaxError("for loop requires a variable to loop with", forLoop.position)) - } else { when (loopvar.datatype) { DataType.UBYTE -> { @@ -319,6 +318,18 @@ internal class AstChecker(private val program: Program, return subroutine } + override fun visit(repeatLoop: RepeatLoop): IStatement { + if(repeatLoop.untilCondition.referencesIdentifiers("A", "X", "Y")) + printWarning("using a register in the loop condition is risky (it could get clobbered)", repeatLoop.untilCondition.position) + return super.visit(repeatLoop) + } + + override fun visit(whileLoop: WhileLoop): IStatement { + if(whileLoop.condition.referencesIdentifiers("A", "X", "Y")) + printWarning("using a register in the loop condition is risky (it could get clobbered)", whileLoop.condition.position) + return super.visit(whileLoop) + } + /** * Assignment target must be register, or a variable name * Also check data type compatibility and number of values @@ -450,7 +461,7 @@ internal class AstChecker(private val program: Program, } // the initializer value can't refer to the variable itself (recursive definition) - if(decl.value?.referencesIdentifier(decl.name) == true || decl.arraysize?.index?.referencesIdentifier(decl.name) == true) { + if(decl.value?.referencesIdentifiers(decl.name) == true || decl.arraysize?.index?.referencesIdentifiers(decl.name) == true) { err("recursive var declaration") } diff --git a/compiler/src/prog8/optimizer/ConstantFolding.kt b/compiler/src/prog8/optimizer/ConstantFolding.kt index d8456ef31..51f58f6ff 100644 --- a/compiler/src/prog8/optimizer/ConstantFolding.kt +++ b/compiler/src/prog8/optimizer/ConstantFolding.kt @@ -28,7 +28,7 @@ class ConstantFolding(private val program: Program) : IAstModifyingVisitor { override fun visit(decl: VarDecl): IStatement { // the initializer value can't refer to the variable itself (recursive definition) - if(decl.value?.referencesIdentifier(decl.name) == true || decl.arraysize?.index?.referencesIdentifier(decl.name) == true) { + if(decl.value?.referencesIdentifiers(decl.name) == true || decl.arraysize?.index?.referencesIdentifiers(decl.name) == true) { errors.add(ExpressionError("recursive var declaration", decl.position)) return decl } diff --git a/examples/test.p8 b/examples/test.p8 index 13f7f63ff..b979d352b 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -8,35 +8,18 @@ Y=0 ubyte aa =0 - while Y<10 { - rsave() - c64scr.print_ub(Y) - c64.CHROUT(',') - rrestore() - Y++ - } - c64.CHROUT('!') - c64.CHROUT('!') - c64.CHROUT('\n') + Y=10 -; repeat { -; c64scr.print_ub(A) -; c64.CHROUT(',') -; A-- -; } until A<5 -; -; c64.CHROUT('!') -; c64.CHROUT('!') -; c64.CHROUT('\n') -; -; for A in 0 to 4 { -; for Y in 0 to 3 { -; c64scr.print_ub(A) -; c64.CHROUT(',') -; c64scr.print_ub(Y) -; c64.CHROUT('\n') -; } -; } + for A in 0 to 4 { + for Y in 0 to 3 { + rsave() + c64scr.print_ub(A) + c64.CHROUT(',') + c64scr.print_ub(Y) + c64.CHROUT('\n') + rrestore() + } + } } }