From d76547ead4d89ac67774ced696f62a63692c94f6 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sat, 11 Mar 2023 14:55:13 +0100 Subject: [PATCH] don't crash on certain undefined symbols, give proper error instead Also the error handlers in unit tests now de-duplicate messages just like the compiler itself does --- .../codegen/cpu6502/ExpressionsAsmGen.kt | 2 +- .../cpu6502/assignment/AssignmentAsmGen.kt | 8 +++--- codeGenCpu6502/test/Dummies.kt | 8 ++++-- codeGenIntermediate/test/Dummies.kt | 8 ++++-- compiler/src/prog8/compiler/Compiler.kt | 2 +- .../compiler/astprocessing/AstChecker.kt | 17 +++++++----- .../astprocessing/AstIdentifiersChecker.kt | 4 ++- compiler/test/TestScoping.kt | 20 +++++++++----- compiler/test/ast/TestVariousCompilerAst.kt | 12 +++++++++ .../test/codegeneration/TestVariousCodeGen.kt | 27 +++++++++++++++++++ .../test/helpers/ErrorReporterForTests.kt | 8 ++++-- docs/source/todo.rst | 1 - 12 files changed, 90 insertions(+), 27 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt index fd8d97893..0bd0d43c9 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt @@ -34,7 +34,7 @@ internal class ExpressionsAsmGen(private val program: PtProgram, is PtFunctionCall -> translateFunctionCallResultOntoStack(expression) is PtBuiltinFunctionCall -> asmgen.translateBuiltinFunctionCallExpression(expression, true, null) is PtContainmentCheck -> throw AssemblyError("containment check as complex expression value is not supported") - is PtArray, is PtString -> throw AssemblyError("no asm gen for string/array literal value assignment - should have been replaced by a variable") + is PtArray, is PtString -> throw AssemblyError("string/array literal value assignment should have been replaced by a variable") is PtRange -> throw AssemblyError("range expression should have been changed into array values") is PtMachineRegister -> throw AssemblyError("machine register ast node should not occur in 6502 codegen it is for IR code") else -> TODO("missing expression asmgen for $expression") diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index f2f428617..271a15283 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -1628,7 +1628,7 @@ internal class AssignmentAsmGen(private val program: PtProgram, """) } TargetStorageKind.MEMORY -> { - throw AssemblyError("no asm gen for assign wordvar $sourceName to memory ${target.memory}") + throw AssemblyError("assign word to memory ${target.memory} should have gotten a typecast") } TargetStorageKind.ARRAY -> { target.array!! @@ -2348,7 +2348,7 @@ internal class AssignmentAsmGen(private val program: PtProgram, } } TargetStorageKind.MEMORY -> { - throw AssemblyError("no asm gen for assign word $word to memory ${target.memory}") + throw AssemblyError("assign word to memory ${target.memory} should have gotten a typecast") } TargetStorageKind.ARRAY -> { asmgen.loadScaledArrayIndexIntoRegister(target.array!!, DataType.UWORD, CpuRegister.Y) @@ -2737,7 +2737,7 @@ internal class AssignmentAsmGen(private val program: PtProgram, asmgen.out(" lda #0 | sta ${wordtarget.asmVarname}+1") } TargetStorageKind.ARRAY -> { - throw AssemblyError("no asm gen for assign memory byte at $address to array ${wordtarget.asmVarname}") + throw AssemblyError("no asm gen for assign memory byte at $address to word array ${wordtarget.asmVarname}") } TargetStorageKind.REGISTER -> when(wordtarget.register!!) { RegisterOrPair.AX -> asmgen.out(" ldx #0 | lda ${address.toHex()}") @@ -2765,7 +2765,7 @@ internal class AssignmentAsmGen(private val program: PtProgram, asmgen.out(" lda #0 | sta ${wordtarget.asmVarname}+1") } TargetStorageKind.ARRAY -> { - throw AssemblyError("no asm gen for assign memory byte $identifier to array ${wordtarget.asmVarname} ") + throw AssemblyError("no asm gen for assign memory byte $identifier to word array ${wordtarget.asmVarname} ") } TargetStorageKind.REGISTER -> { asmgen.loadByteFromPointerIntoA(identifier) diff --git a/codeGenCpu6502/test/Dummies.kt b/codeGenCpu6502/test/Dummies.kt index 132037f1a..3351be558 100644 --- a/codeGenCpu6502/test/Dummies.kt +++ b/codeGenCpu6502/test/Dummies.kt @@ -34,11 +34,15 @@ internal class ErrorReporterForTests(private val throwExceptionAtReportIfErrors: val warnings = mutableListOf() override fun err(msg: String, position: Position) { - errors.add("${position.toClickableStr()} $msg") + val text = "${position.toClickableStr()} $msg" + if(text !in errors) + errors.add(text) } override fun warn(msg: String, position: Position) { - warnings.add("${position.toClickableStr()} $msg") + val text = "${position.toClickableStr()} $msg" + if(text !in warnings) + warnings.add(text) } override fun noErrors(): Boolean = errors.isEmpty() diff --git a/codeGenIntermediate/test/Dummies.kt b/codeGenIntermediate/test/Dummies.kt index 1bd5157f2..37b7dc30c 100644 --- a/codeGenIntermediate/test/Dummies.kt +++ b/codeGenIntermediate/test/Dummies.kt @@ -32,11 +32,15 @@ internal class ErrorReporterForTests(private val throwExceptionAtReportIfErrors: val warnings = mutableListOf() override fun err(msg: String, position: Position) { - errors.add("${position.toClickableStr()} $msg") + val text = "${position.toClickableStr()} $msg" + if(text !in errors) + errors.add(text) } override fun warn(msg: String, position: Position) { - warnings.add("${position.toClickableStr()} $msg") + val text = "${position.toClickableStr()} $msg" + if(text !in warnings) + warnings.add(text) } override fun noErrors(): Boolean = errors.isEmpty() diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index 9dfff5419..d8f5a13a6 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -405,7 +405,7 @@ private fun createAssemblyAndAssemble(program: PtProgram, else if (compilerOptions.compTarget.name == VMTarget.NAME) VmCodeGen() else - throw NotImplementedError("no asm generator for cpu ${compilerOptions.compTarget.machine.cpu}") + throw NotImplementedError("no code generator for cpu ${compilerOptions.compTarget.machine.cpu}") val stMaker = SymbolTableMaker(program, compilerOptions) val symbolTable = stMaker.make() diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 0d0c23406..8284a8bc5 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -60,11 +60,16 @@ internal class AstChecker(private val program: Program, } override fun visit(identifier: IdentifierReference) { - val target = identifier.targetVarDecl(program) - if(target != null && target.origin==VarDeclOrigin.SUBROUTINEPARAM) { - if(target.definingSubroutine!!.isAsmSubroutine) { - if(target.definingSubroutine!!.parameters.any { it.name == identifier.nameInSource.last() }) - errors.err("cannot refer to parameter of asmsub by name", identifier.position) + val stmt = identifier.targetStatement(program) + if(stmt==null) + errors.err("undefined symbol: ${identifier.nameInSource.joinToString(".")}", identifier.position) + else { + val target = stmt as? VarDecl + if (target != null && target.origin == VarDeclOrigin.SUBROUTINEPARAM) { + if (target.definingSubroutine!!.isAsmSubroutine) { + if (target.definingSubroutine!!.parameters.any { it.name == identifier.nameInSource.last() }) + errors.err("cannot refer to parameter of asmsub by name", identifier.position) + } } } } @@ -1323,7 +1328,7 @@ internal class AstChecker(private val program: Program, else errors.err("cannot call that: ${target.nameInSource.joinToString(".")}", target.position) } - null -> errors.err("undefined function or subroutine: ${target.nameInSource.joinToString(".")}", target.position) + null -> errors.err("undefined symbol: ${target.nameInSource.joinToString(".")}", target.position) else -> errors.err("cannot call that: ${target.nameInSource.joinToString(".")}", target.position) } return null diff --git a/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt index 2580a9146..811915071 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt @@ -13,7 +13,9 @@ import prog8.code.core.IErrorReporter import prog8.code.core.Position import prog8.code.target.VMTarget - +/** + * This checks for naming conflicts, not for correct symbol references yet. + */ internal class AstIdentifiersChecker(private val errors: IErrorReporter, private val program: Program, private val compTarget: ICompilationTarget diff --git a/compiler/test/TestScoping.kt b/compiler/test/TestScoping.kt index 8e7596d0c..5b2386b0d 100644 --- a/compiler/test/TestScoping.kt +++ b/compiler/test/TestScoping.kt @@ -413,7 +413,16 @@ class TestScoping: FunSpec({ """ val errors = ErrorReporterForTests() compileText(C64Target(), false, text, writeAssembly = false, errors = errors) shouldBe null - println(errors.errors) + /* +There are 4 errors and 3 warnings. +ERROR name conflict 'start', also defined... +ERROR name conflict 'var1Warn', also defined... +ERROR name conflict 'main', also defined... +ERROR name conflict 'internalOk', also defined... +WARN name 'var1Warn' shadows occurrence at... +WARN name 'var1Warn' shadows occurrence at... +WARN name 'var1Warn' shadows occurrence at... + */ errors.warnings.size shouldBe 3 errors.warnings[0] shouldContain "var1Warn" errors.warnings[0] shouldContain "shadows" @@ -424,7 +433,7 @@ class TestScoping: FunSpec({ errors.warnings[2] shouldContain "var1Warn" errors.warnings[2] shouldContain "shadows" errors.warnings[2] shouldContain "line 3" - errors.errors.size shouldBe 5 + errors.errors.size shouldBe 4 errors.errors[0] shouldContain "name conflict" errors.errors[0] shouldContain "start" errors.errors[0] shouldContain "line 5" @@ -435,10 +444,7 @@ class TestScoping: FunSpec({ errors.errors[2] shouldContain "main" errors.errors[2] shouldContain "line 2" errors.errors[3] shouldContain "name conflict" - errors.errors[3] shouldContain "start" - errors.errors[3] shouldContain "line 5" - errors.errors[4] shouldContain "name conflict" - errors.errors[4] shouldContain "internalOk" - errors.errors[4] shouldContain "line 11" + errors.errors[3] shouldContain "internalOk" + errors.errors[3] shouldContain "line 11" } }) diff --git a/compiler/test/ast/TestVariousCompilerAst.kt b/compiler/test/ast/TestVariousCompilerAst.kt index 94d7ae04e..d259ed25e 100644 --- a/compiler/test/ast/TestVariousCompilerAst.kt +++ b/compiler/test/ast/TestVariousCompilerAst.kt @@ -201,5 +201,17 @@ main { value2.left shouldBe instanceOf() (value2.right as NumericLiteral).number shouldBe 0.0 } + + test("const pointer variable indexing works") { + val src=""" +main { + sub start() { + const uword pointer=$1000 + cx16.r0L = pointer[2] + } +} +""" + compileText(C64Target(), optimize=false, src, writeAssembly=false) shouldNotBe null + } }) diff --git a/compiler/test/codegeneration/TestVariousCodeGen.kt b/compiler/test/codegeneration/TestVariousCodeGen.kt index 80ef7752a..a534f7622 100644 --- a/compiler/test/codegeneration/TestVariousCodeGen.kt +++ b/compiler/test/codegeneration/TestVariousCodeGen.kt @@ -4,6 +4,7 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.ints.shouldBeGreaterThan import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe +import io.kotest.matchers.string.shouldContain import io.kotest.matchers.string.shouldStartWith import io.kotest.matchers.types.instanceOf import prog8.code.ast.PtArrayIndexer @@ -11,6 +12,7 @@ import prog8.code.ast.PtAssignment import prog8.code.ast.PtVariable import prog8.code.core.DataType import prog8.code.target.C64Target +import prog8tests.helpers.ErrorReporterForTests import prog8tests.helpers.compileText class TestVariousCodeGen: FunSpec({ @@ -109,4 +111,29 @@ main { }""" compileText(C64Target(), false, text, writeAssembly = true) shouldNotBe null } + + test("assigning memory byte into array works") { + val text=""" +main { + sub start() { + uword factor1 + ubyte[3] @shared factor124 + factor124[0] = @(factor1) + } +}""" + compileText(C64Target(), false, text, writeAssembly = true) shouldNotBe null + } + + test("reading memory from unknown var gives proper error") { + val text=""" +main { + sub start() { + cx16.r0L = @(doesnotexist) + } +}""" + val errors = ErrorReporterForTests() + compileText(C64Target(), false, text, writeAssembly = true, errors = errors) + errors.errors.size shouldBe 1 + errors.errors[0] shouldContain "undefined symbol: doesnotexist" + } }) \ No newline at end of file diff --git a/compiler/test/helpers/ErrorReporterForTests.kt b/compiler/test/helpers/ErrorReporterForTests.kt index b4e93c483..c2d8806bb 100644 --- a/compiler/test/helpers/ErrorReporterForTests.kt +++ b/compiler/test/helpers/ErrorReporterForTests.kt @@ -10,11 +10,15 @@ internal class ErrorReporterForTests(private val throwExceptionAtReportIfErrors: val warnings = mutableListOf() override fun err(msg: String, position: Position) { - errors.add("${position.toClickableStr()} $msg") + val text = "${position.toClickableStr()} $msg" + if(text !in errors) + errors.add(text) } override fun warn(msg: String, position: Position) { - warnings.add("${position.toClickableStr()} $msg") + val text = "${position.toClickableStr()} $msg" + if(text !in warnings) + warnings.add(text) } override fun noErrors(): Boolean = errors.isEmpty() diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 331132f1e..a3ccbf173 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,7 +3,6 @@ TODO For next minor release ^^^^^^^^^^^^^^^^^^^^^^ -- fix Github issue with array problems https://github.com/irmen/prog8/issues/99 - fix IR/VM: animals.p8 example is borked, it jumps straight to a suggestion and then somehow doesn't print the animal name correctly in the first question, and exits after 1 animal instead of looping this has happened after v8.7: caused by c21913a6 ir: keep order of children in block 22-11-2022