diff --git a/compiler/src/prog8/ast/base/Extensions.kt b/compiler/src/prog8/ast/base/Extensions.kt index f4464d25d..794ccfb29 100644 --- a/compiler/src/prog8/ast/base/Extensions.kt +++ b/compiler/src/prog8/ast/base/Extensions.kt @@ -4,6 +4,7 @@ import prog8.ast.Module import prog8.ast.Program import prog8.ast.processing.* import prog8.compiler.CompilationOptions +import prog8.compiler.target.c64.codegen2.AnonymousScopeVarsCleanup import prog8.optimizer.FlattenAnonymousScopesAndRemoveNops @@ -28,6 +29,13 @@ internal fun Program.checkValid(compilerOptions: CompilationOptions) { } +internal fun Program.anonscopeVarsCleanup() { + val mover = AnonymousScopeVarsCleanup(this) + mover.visit(this) + printErrors(mover.result(), name) +} + + internal fun Program.reorderStatements() { val initvalueCreator = VarInitValueAndAddressOfCreator(namespace, heap) initvalueCreator.visit(this) diff --git a/compiler/src/prog8/ast/processing/AstIdentifiersChecker.kt b/compiler/src/prog8/ast/processing/AstIdentifiersChecker.kt index 438c868e8..0779c567b 100644 --- a/compiler/src/prog8/ast/processing/AstIdentifiersChecker.kt +++ b/compiler/src/prog8/ast/processing/AstIdentifiersChecker.kt @@ -118,6 +118,14 @@ internal class AstIdentifiersChecker(private val program: Program) : IAstModifyi if (existing != null && existing !== subroutine) nameError(subroutine.name, subroutine.position, existing) + // does the parameter redefine a variable declared elsewhere? + for(param in subroutine.parameters) { + val existingVar = subroutine.lookup(listOf(param.name), subroutine) + if (existingVar != null && existingVar.parent !== subroutine) { + nameError(param.name, param.position, existingVar) + } + } + // check that there are no local variables, labels, or other subs that redefine the subroutine's parameters val symbolsInSub = subroutine.allDefinedSymbols() val namesInSub = symbolsInSub.map{ it.first }.toSet() @@ -188,14 +196,18 @@ internal class AstIdentifiersChecker(private val program: Program) : IAstModifyi vardecl.linkParents(forLoop.body) forLoop.body.statements.add(0, vardecl) loopVar.parent = forLoop.body // loopvar 'is defined in the body' + } else if(existing.parent!==forLoop && existing.parent.parent!==forLoop) { + checkResult.add(NameError("for loop var was already defined at ${existing.position}", loopVar.position)) } } + val validName = forLoop.body.name.replace("<", "").replace(">", "").replace("-", "") + val loopvarName = "prog8_loopvar_$validName" if (forLoop.iterable !is RangeExpr) { - val existing = if (forLoop.body.containsNoCodeNorVars()) null else forLoop.body.lookup(listOf(ForLoop.iteratorLoopcounterVarname), forLoop.body.statements.first()) + val existing = if (forLoop.body.containsNoCodeNorVars()) null else forLoop.body.lookup(listOf(loopvarName), forLoop.body.statements.first()) if (existing == null) { // create loop iteration counter variable (without value, to avoid an assignment) - val vardecl = VarDecl(VarDeclType.VAR, DataType.UBYTE, ZeropageWish.PREFER_ZEROPAGE, null, ForLoop.iteratorLoopcounterVarname, null, null, + val vardecl = VarDecl(VarDeclType.VAR, DataType.UBYTE, ZeropageWish.PREFER_ZEROPAGE, null, loopvarName, null, null, isArray = false, autogeneratedDontRemove = true, position = loopVar.position) vardecl.linkParents(forLoop.body) forLoop.body.statements.add(0, vardecl) diff --git a/compiler/src/prog8/ast/statements/AstStatements.kt b/compiler/src/prog8/ast/statements/AstStatements.kt index 7e8dc41f4..f6149f384 100644 --- a/compiler/src/prog8/ast/statements/AstStatements.kt +++ b/compiler/src/prog8/ast/statements/AstStatements.kt @@ -710,10 +710,6 @@ class ForLoop(val loopRegister: Register?, override fun toString(): String { return "ForLoop(loopVar: $loopVar, loopReg: $loopRegister, iterable: $iterable, pos=$position)" } - - companion object { - const val iteratorLoopcounterVarname = "prog8forloopcounter" - } } class WhileLoop(var condition: Expression, diff --git a/compiler/src/prog8/compiler/Main.kt b/compiler/src/prog8/compiler/Main.kt index cb061953d..58550b3ca 100644 --- a/compiler/src/prog8/compiler/Main.kt +++ b/compiler/src/prog8/compiler/Main.kt @@ -5,7 +5,6 @@ import prog8.ast.Program import prog8.ast.base.* import prog8.ast.statements.Directive import prog8.compiler.target.c64.MachineDefinition -import prog8.compiler.target.c64.codegen2.AnonymousScopeVarsCleanup import prog8.compiler.target.c64.codegen2.AsmGen2 import prog8.optimizer.constantFold import prog8.optimizer.optimizeStatements @@ -92,7 +91,7 @@ fun compileProgram(filepath: Path, if(writeAssembly) { // asm generation directly from the Ast, no need for intermediate code val zeropage = MachineDefinition.C64Zeropage(compilerOptions) - AnonymousScopeVarsCleanup.moveVarsToSubroutine(programAst) + programAst.anonscopeVarsCleanup() val assembly = AsmGen2(programAst, compilerOptions, zeropage).compileToAssembly(optimize) assembly.assemble(compilerOptions) programName = assembly.name diff --git a/compiler/src/prog8/compiler/target/c64/codegen2/AnonymousScopeVarsCleanup.kt b/compiler/src/prog8/compiler/target/c64/codegen2/AnonymousScopeVarsCleanup.kt index 502dfc0ec..d03f5d768 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen2/AnonymousScopeVarsCleanup.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen2/AnonymousScopeVarsCleanup.kt @@ -1,82 +1,47 @@ package prog8.compiler.target.c64.codegen2 import prog8.ast.Program -import prog8.ast.base.FatalAstException -import prog8.ast.expressions.Expression -import prog8.ast.expressions.IdentifierReference +import prog8.ast.base.AstException +import prog8.ast.base.NameError import prog8.ast.processing.IAstModifyingVisitor import prog8.ast.statements.AnonymousScope import prog8.ast.statements.Statement import prog8.ast.statements.VarDecl -import java.util.* class AnonymousScopeVarsCleanup(val program: Program): IAstModifyingVisitor { - companion object { - fun moveVarsToSubroutine(programAst: Program) { - val cleanup = AnonymousScopeVarsCleanup(programAst) - cleanup.visit(programAst) + private val checkResult: MutableList = mutableListOf() + private val varsToMove: MutableMap> = mutableMapOf() - for((scope, decls) in cleanup.varsToMove) { - decls.forEach { scope.remove(it) } - val sub = scope.definingSubroutine()!! - val existingVariables = sub.statements.filterIsInstance().map { it.name }.toSet() - sub.statements.addAll(0, decls) - decls.forEach { - it.parent=sub - if(it.name in existingVariables) { - throw FatalAstException("variable ${it.name} already exists in ${sub.name}") - } + fun result(): List { + return checkResult + } + + override fun visit(program: Program) { + varsToMove.clear() + super.visit(program) + for((scope, decls) in varsToMove) { + val sub = scope.definingSubroutine()!! + val existingVariables = sub.statements.filterIsInstance().map { it.name }.toSet() + var conflicts = false + decls.forEach { + if (it.name in existingVariables) { + checkResult.add(NameError("variable ${it.name} already exists in subroutine ${sub.name}", it.position)) + conflicts = true } } + if (!conflicts) { + decls.forEach { scope.remove(it) } + sub.statements.addAll(0, decls) + decls.forEach { it.parent = sub } + } } } - private val varsToMove: MutableMap> = mutableMapOf() - - private val currentAnonScope: Stack = Stack() - override fun visit(scope: AnonymousScope): Statement { - currentAnonScope.push(scope) val scope2 = super.visit(scope) as AnonymousScope - currentAnonScope.pop() val vardecls = scope2.statements.filterIsInstance() varsToMove[scope2] = vardecls return scope2 } - - private fun nameprefix(scope: AnonymousScope) = scope.name.replace("<", "").replace(">", "").replace("-", "") + "_" - - override fun visit(decl: VarDecl): Statement { - val decl2 = super.visit(decl) as VarDecl - if(currentAnonScope.isEmpty()) - return decl2 - return decl2.withPrefixedName(nameprefix(currentAnonScope.peek())) - } - - override fun visit(identifier: IdentifierReference): Expression { - val ident = super.visit(identifier) - if(ident !is IdentifierReference) - return ident - if(currentAnonScope.isEmpty()) - return ident - val vardecl = ident.targetVarDecl(program.namespace) - return if(vardecl!=null && vardecl.definingScope() === ident.definingScope()) { - // prefix the variable name reference that is defined inside the anon scope - ident.withPrefixedName(nameprefix(currentAnonScope.peek())) - } else { - ident - } - } - /* -; @todo FIX Symbol lookup over anon scopes -; sub start() { -; for ubyte i in 0 to 10 { -; word rz = 4 -; if rz >= 1 { -; word persp = rz+1 -; } -; } -; } - */ } diff --git a/compiler/src/prog8/compiler/target/c64/codegen2/AsmGen2.kt b/compiler/src/prog8/compiler/target/c64/codegen2/AsmGen2.kt index 347fd428b..9a96f4cef 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen2/AsmGen2.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen2/AsmGen2.kt @@ -925,7 +925,7 @@ internal class AsmGen2(val program: Program, } private fun translate(scope: AnonymousScope) { - // note: the variables defined in an anonymous scope are moved to their defining subroutine's scope + // note: the variables defined in an anonymous scope have been moved to their defining subroutine's scope scope.statements.forEach{ translate(it) } } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 5edaa47e3..0128e9efa 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -2,6 +2,28 @@ TODO ==== + +Fixes +^^^^^ +variable naming issue:: + + main { + + sub start() { + for A in 0 to 10 { + ubyte note1 = 44 + Y+=note1 + } + delay(1) + + sub delay(ubyte note1) { ; TODO: redef of note1 above, conflicts because that one was moved to the zeropage + A= note1 + } + } + } + + + Memory Block Operations integrated in language? ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/examples/bdmusic.p8 b/examples/bdmusic.p8 index 5a094a9e6..c902f0780 100644 --- a/examples/bdmusic.p8 +++ b/examples/bdmusic.p8 @@ -18,10 +18,10 @@ sub start() { while(true) { for uword note in notes { - ubyte n1 = lsb(note) - ubyte n2 = msb(note) - c64.FREQ1 = music_freq_table[n1] ; set lo+hi freq of voice 1 - c64.FREQ2 = music_freq_table[n2] ; set lo+hi freq of voice 2 + ubyte note1 = lsb(note) + ubyte note2 = msb(note) + c64.FREQ1 = music_freq_table[note1] ; set lo+hi freq of voice 1 + c64.FREQ2 = music_freq_table[note2] ; set lo+hi freq of voice 2 ; retrigger voice 1 and 2 ADSR c64.CR1 = waveform <<4 | 0 @@ -29,7 +29,7 @@ sub start() { c64.CR1 = waveform <<4 | 1 c64.CR2 = waveform <<4 | 1 - print_notes(n1, n2) + print_notes(note1, note2) delay() } } diff --git a/examples/test.p8 b/examples/test.p8 index 851a5f84a..193464552 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -6,42 +6,20 @@ main { sub start() { -; float fl = 123.4567 -; c64flt.print_f(round(fl)) -; c64.CHROUT('\n') -; c64flt.print_f(round(fl)) -; c64.CHROUT('\n') -; c64flt.print_f(round(fl)) -; c64.CHROUT('\n') -; c64flt.print_f(ceil(fl)) -; c64.CHROUT('\n') -; c64flt.print_f(ceil(fl)) -; c64.CHROUT('\n') -; c64flt.print_f(ceil(fl)) -; c64.CHROUT('\n') -; c64flt.print_f(floor(fl)) -; c64.CHROUT('\n') -; c64flt.print_f(floor(fl)) -; c64.CHROUT('\n') -; c64flt.print_f(floor(fl)) -; c64.CHROUT('\n') -; @($040a)=X -; return + delay1(1) + delay2(1) + delay3(1) - while true { - float clock_seconds = ((mkword(c64.TIME_LO, c64.TIME_MID) as float) + (c64.TIME_HI as float)*65536.0) / 60 - float hours = floor(clock_seconds / 3600) - clock_seconds -= hours*3600 - float minutes = floor(clock_seconds / 60) - clock_seconds = floor(clock_seconds - minutes * 60.0) - - c64scr.print("system time in ti$ is ") - c64flt.print_f(hours) - c64.CHROUT(':') - c64flt.print_f(minutes) - c64.CHROUT(':') - c64flt.print_f(clock_seconds) - c64.CHROUT('\n') + sub delay1(ubyte note1) { + A= note1 } + sub delay2(ubyte note1) { + A= note1 + } + sub delay3(ubyte note1) { + A= note1 + } + } + }