From d58f9f56c45380d9c124d96e844575b30a3efb96 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 24 Nov 2024 19:07:58 +0100 Subject: [PATCH] tests for register args for normal subs some warnings demoted into infos --- .../src/prog8/codegen/cpu6502/IfElseAsmGen.kt | 2 +- .../prog8/optimizer/ExpressionSimplifier.kt | 8 +- .../compiler/astprocessing/AstChecker.kt | 2 +- .../compiler/astprocessing/AstPreprocessor.kt | 9 +- .../astprocessing/BeforeAsmTypecastCleaner.kt | 4 +- .../astprocessing/LiteralsToAutoVars.kt | 1 - .../compiler/astprocessing/TypecastsAdder.kt | 2 +- compiler/test/TestNumbers.kt | 6 +- compiler/test/ast/TestAstChecks.kt | 83 ++++++++++++++++++- docs/source/todo.rst | 5 -- examples/test.p8 | 37 ++++----- 11 files changed, 113 insertions(+), 46 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/IfElseAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/IfElseAsmGen.kt index b9459c109..6944db153 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/IfElseAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/IfElseAsmGen.kt @@ -347,7 +347,7 @@ internal class IfElseAsmGen(private val program: PtProgram, else translateIfElseBodies("beq", stmt) } else { - errors.warn("SLOW FALLBACK FOR 'IF' CODEGEN - ask for support", stmt.position) // should not occur ;-) + errors.info("SLOW FALLBACK FOR 'IF' CODEGEN - ask for support", stmt.position) // should not occur ;-) assignConditionValueToRegisterAndTest(stmt.condition) if(jumpAfterIf!=null) translateJumpElseBodies("bne", "beq", jumpAfterIf, stmt.elseScope) diff --git a/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt b/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt index 4dd5dce32..532330256 100644 --- a/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt +++ b/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt @@ -796,13 +796,13 @@ class ExpressionSimplifier(private val program: Program, private val errors: IEr when (val targetDt = targetIDt.getOr(DataType.UNDEFINED)) { DataType.UBYTE, DataType.BYTE -> { if (amount >= 8) { - errors.warn("shift always results in 0", expr.position) + errors.info("shift always results in 0", expr.position) return NumericLiteral(targetDt, 0.0, expr.position) } } DataType.UWORD -> { if (amount >= 16) { - errors.warn("shift always results in 0", expr.position) + errors.info("shift always results in 0", expr.position) return NumericLiteral(targetDt, 0.0, expr.position) } else if(amount==8) { @@ -819,7 +819,7 @@ class ExpressionSimplifier(private val program: Program, private val errors: IEr } DataType.WORD -> { if (amount >= 16) { - errors.warn("shift always results in 0", expr.position) + errors.info("shift always results in 0", expr.position) return NumericLiteral(targetDt, 0.0, expr.position) } else if(amount==8) { @@ -856,7 +856,7 @@ class ExpressionSimplifier(private val program: Program, private val errors: IEr when (idt.getOr(DataType.UNDEFINED)) { DataType.UBYTE -> { if (amount >= 8) { - errors.warn("shift always results in 0", expr.position) + errors.info("shift always results in 0", expr.position) return NumericLiteral.optimalInteger(0, expr.position) } } diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 955b7aa95..8ac67496d 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -372,7 +372,7 @@ internal class AstChecker(private val program: Program, err("cannot redefine a built-in function") if(subroutine.parameters.size>6 && !subroutine.isAsmSubroutine && !subroutine.definingBlock.isInLibrary) - errors.warn("subroutine has a large number of parameters, this is slow if called often", subroutine.position) + errors.info("subroutine has a large number of parameters, this is slow if called often", subroutine.position) val uniqueNames = subroutine.parameters.asSequence().map { it.name }.toSet() if(uniqueNames.size!=subroutine.parameters.size) diff --git a/compiler/src/prog8/compiler/astprocessing/AstPreprocessor.kt b/compiler/src/prog8/compiler/astprocessing/AstPreprocessor.kt index 1c21ac89c..812dab1fe 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstPreprocessor.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstPreprocessor.kt @@ -235,9 +235,12 @@ class AstPreprocessor(val program: Program, registerParams .filter { it.name !in namesInSub && it.name !in existingAliases } .forEach { - val regname = it.registerOrPair!!.asScopedNameVirtualReg(it.type) - var alias = Alias(it.name, IdentifierReference(regname, it.position), it.position) - mods += IAstModification.InsertFirst(alias, subroutine) + if (it.registerOrPair in Cx16VirtualRegisters) { + val regname = it.registerOrPair!!.asScopedNameVirtualReg(it.type) + var alias = Alias(it.name, IdentifierReference(regname, it.position), it.position) + mods += IAstModification.InsertFirst(alias, subroutine) + } else + errors.err("can only use R0-R15 as register param for normal subroutines", it.position) } } return mods diff --git a/compiler/src/prog8/compiler/astprocessing/BeforeAsmTypecastCleaner.kt b/compiler/src/prog8/compiler/astprocessing/BeforeAsmTypecastCleaner.kt index c78c349e5..70ddecc8e 100644 --- a/compiler/src/prog8/compiler/astprocessing/BeforeAsmTypecastCleaner.kt +++ b/compiler/src/prog8/compiler/astprocessing/BeforeAsmTypecastCleaner.kt @@ -101,9 +101,9 @@ internal class BeforeAsmTypecastCleaner(val program: Program, if(shifts!=null) { val dt = expr.left.inferType(program) if(dt.istype(DataType.UBYTE) && shifts.number>=8.0) - errors.warn("shift always results in 0", expr.position) + errors.info("shift always results in 0", expr.position) if(dt.istype(DataType.UWORD) && shifts.number>=16.0) - errors.warn("shift always results in 0", expr.position) + errors.info("shift always results in 0", expr.position) if(shifts.number<=255.0 && shifts.type in WordDatatypes) { val byteVal = NumericLiteral(DataType.UBYTE, shifts.number, shifts.position) return listOf(IAstModification.ReplaceNode(expr.right, byteVal, expr)) diff --git a/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt b/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt index a5e60f16c..6f07a587c 100644 --- a/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt +++ b/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt @@ -84,7 +84,6 @@ internal class LiteralsToAutoVars(private val program: Program, private val erro val decl = elt.targetVarDecl(program) if(decl!=null && decl.splitArray) { // you can't take the adress of a split-word array. - // instead of giving a fatal error, we remove the // instead of a fatal error, we give a warning and turn it back into a regular array. errors.warn("cannot take address of split word array - the array is turned back into a regular word array", decl.position) val normalArray = makeNormalArrayFromSplit(decl) diff --git a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt index e32c0c36f..d5dce7fb8 100644 --- a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt +++ b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt @@ -263,7 +263,7 @@ class TypecastsAdder(val program: Program, val options: CompilationOptions, val // warn about any implicit type casts to Float, because that may not be intended if(typecast.implicit && typecast.type.oneOf(DataType.FLOAT, DataType.ARRAY_F)) { if(options.floats) - errors.warn("integer implicitly converted to float. Suggestion: use float literals, add an explicit cast, or revert to integer arithmetic", typecast.position) + errors.info("integer implicitly converted to float. Suggestion: use float literals, add an explicit cast, or revert to integer arithmetic", typecast.position) else errors.err("integer implicitly converted to float but floating point is not enabled via options", typecast.position) } diff --git a/compiler/test/TestNumbers.kt b/compiler/test/TestNumbers.kt index e769d1501..2feb2b4d9 100644 --- a/compiler/test/TestNumbers.kt +++ b/compiler/test/TestNumbers.kt @@ -131,9 +131,9 @@ class TestNumbers: FunSpec({ val errors = ErrorReporterForTests(keepMessagesAfterReporting = true) compileText(C64Target(), true, src, writeAssembly = false, errors=errors) shouldNotBe null errors.errors.size shouldBe 0 - errors.warnings.size shouldBe 2 - errors.warnings[0] shouldContain "converted to float" - errors.warnings[1] shouldContain "converted to float" + errors.infos.size shouldBe 2 + errors.infos[0] shouldContain "converted to float" + errors.infos[1] shouldContain "converted to float" } test("implicit float conversion error if not enabled") { diff --git a/compiler/test/ast/TestAstChecks.kt b/compiler/test/ast/TestAstChecks.kt index e2847134f..279d821fd 100644 --- a/compiler/test/ast/TestAstChecks.kt +++ b/compiler/test/ast/TestAstChecks.kt @@ -28,9 +28,9 @@ class TestAstChecks: FunSpec({ val errors = ErrorReporterForTests(keepMessagesAfterReporting = true) compileText(C64Target(), true, text, writeAssembly = true, errors=errors) shouldNotBe null errors.errors.size shouldBe 0 - errors.warnings.size shouldBe 2 - errors.warnings[0] shouldContain "converted to float" - errors.warnings[1] shouldContain "converted to float" + errors.infos.size shouldBe 2 + errors.infos[0] shouldContain "converted to float" + errors.infos[1] shouldContain "converted to float" } test("can't assign label or subroutine without using address-of") { @@ -273,4 +273,81 @@ main { compileText(C64Target(), false, src, writeAssembly = false) shouldNotBe null compileText(C64Target(), true, src, writeAssembly = false) shouldNotBe null } + + test("reg params cannot be statusflag") { + val src=""" +main { + sub start() { + faulty(false) + } + + sub faulty(bool flag @Pc) { + cx16.r0++ + } +}""" + + // the syntax error is actually thrown by the parser, so we cannot catch it, but we know that there may not be a compilation result + compileText(C64Target(), false, src, writeAssembly = false) shouldBe null + } + + test("reg params cannot be cpu register") { + val src=""" +main { + sub start() { + faulty(42) + } + + sub faulty(byte arg @Y) { + arg++ + } +}""" + + val errors = ErrorReporterForTests() + compileText(C64Target(), false, src, writeAssembly = false, errors = errors) shouldBe null + errors.errors.size shouldBe 1 + errors.errors[0] shouldContain "can only use R0-R15" + } + + test("reg params must be all different") { + val src=""" +main { + sub start() { + faulty3(9999,55) + } + + sub faulty3(uword arg @R1, ubyte arg2 @R1) { + arg += arg2 + } +}""" + + val errors = ErrorReporterForTests() + compileText(C64Target(), false, src, writeAssembly = false, errors = errors) shouldBe null + errors.errors.size shouldBe 1 + errors.errors[0] shouldContain "register is used multiple times" + } + + test("reg params R0-R15 are ok") { + val src=""" +main { + sub start() { + foo(42) + bar(9999,55) + } + + sub foo(ubyte arg @R2) { + arg++ + } + + sub bar(uword arg @R0, ubyte arg2 @R1) { + arg += arg2 + } +}""" + + val errors = ErrorReporterForTests(keepMessagesAfterReporting = true) + compileText(C64Target(), false, src, writeAssembly = false, errors = errors) shouldNotBe null + errors.errors.size shouldBe 0 + errors.warnings.size shouldBe 2 + errors.warnings[0] shouldContain "footgun" + errors.warnings[1] shouldContain "footgun" + } }) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 430e0e8dc..751033848 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -2,12 +2,9 @@ TODO ==== document the @R0 - @R15 register support for normal subroutine parameters (footgun!) -add unit tests for it too. make a compiler switch to disable footgun warnings -turn some existing warnings into INFO - what to do with bankof(): keep it? add another syntax like \`value or ^value to get the bank byte? add a function like addr() or lsw() to complement bnk() in getting easy access to the lower 16 bits of a long integer? -> added unary ^ operator as alternative to bankof() @@ -21,8 +18,6 @@ add a function like addr() or lsw() to complement bnk() in getting easy access t Future Things and Ideas ^^^^^^^^^^^^^^^^^^^^^^^ -- a way to specify subroutine params for normal (non-asmsub) subs to be in r0-r15 registers instead of allocating a new localvariable for them -- the previous should give another footgun warning (the @dirty info should be a footgun warning/info as well). Need a switch to turn off footgun warnings. - something to reduce the need to use fully qualified names all the time. 'with' ? Or 'using '? - on the C64: make the floating point routines @banked so that basic can be permanently banked out even if you use floats? But this will crash when the call is done from program code at $a000+ - Libraries: improve ability to create library files in prog8; for instance there's still stuff injected into the start of the start() routine AND there is separate setup logic going on before calling it. diff --git a/examples/test.p8 b/examples/test.p8 index 3af48fa1e..b18fed581 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,36 +1,29 @@ -%import textio -%zeropage basicsafe - main { sub start() { foo(42) - foo2(42) bar(9999,55) - bar2(9999,55) - txt.nl() +; faulty1(false) + faulty2(42) + faulty3(9999,55) } - sub foo(ubyte arg) { - txt.print_ub(arg) - txt.nl() + sub foo(ubyte arg @R2) { + arg++ } - sub foo2(ubyte arg @R2) { - txt.print_ub(arg) - txt.nl() + sub bar(uword arg @R0, ubyte arg2 @R1) { + arg += arg2 } - sub bar(uword arg, ubyte arg2) { - txt.print_uw(arg) - txt.spc() - txt.print_ub(arg2) - txt.nl() +; sub faulty1(bool flag @Pc) { +; cx16.r0++ +; } + + sub faulty2(byte arg @Y) { + arg++ } - sub bar2(uword arg @R0, ubyte arg2 @R1) { - txt.print_uw(arg) - txt.spc() - txt.print_ub(arg2) - txt.nl() + sub faulty3(uword arg @R1, ubyte arg2 @R1) { + arg += arg2 } }