Apple-2-SW/prog8
1
0
Fork 0
mirror of https://github.com/irmen/prog8.git synced 2024-07-09 17:29:46 +00:00
Code Issues Projects Releases Wiki Activity

fix compiler crash bug due to reused ast expression nodes. Now all (relevant) Nodes have a copy() function to make a clone.

Browse Source
This commit is contained in:
Irmen de Jong 2021-11-17 22:34:34 +01:00
parent 2e0450d7ed
commit dafa0d9138
7 changed files with 114 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt
View File

@ -305,22 +305,22 @@ class StatementOptimizer(private val program: Program,
if(rNum!=null) {
if (op1 == "+" || op1 == "-") {
if (op2 == "+") {
// A = A +/- B + N
// A = A +/- B + N ---> A = A +/- B ; A = A + N
val expr2 = BinaryExpression(binExpr.left, binExpr.operator, rExpr.left, binExpr.position)
val addConstant = Assignment(
assignment.target,
BinaryExpression(binExpr.left, "+", rExpr.right, rExpr.position),
assignment.target.copy(),
BinaryExpression(binExpr.left.copy(), "+", rExpr.right, rExpr.position),
assignment.position
)
return listOf(
IAstModification.ReplaceNode(binExpr, expr2, binExpr.parent),
IAstModification.InsertAfter(assignment, addConstant, parent as IStatementContainer))
} else if (op2 == "-") {
// A = A +/- B - N
// A = A +/- B - N ---> A = A +/- B ; A = A - N
val expr2 = BinaryExpression(binExpr.left, binExpr.operator, rExpr.left, binExpr.position)
val subConstant = Assignment(
assignment.target,
BinaryExpression(binExpr.left, "-", rExpr.right, rExpr.position),
assignment.target.copy(),
BinaryExpression(binExpr.left.copy(), "-", rExpr.right, rExpr.position),
assignment.position
)
return listOf(
@ -375,7 +375,7 @@ class StatementOptimizer(private val program: Program,
repeat(rightCv.toInt()) {
incs.statements.add(PostIncrDecr(assignment.target.copy(), "++", assignment.position))
}
return listOf(IAstModification.ReplaceNode(assignment, incs, parent))
listOf(IAstModification.ReplaceNode(assignment, if(incs.statements.size==1) incs.statements[0] else incs, parent))
}
}
}

39
compiler/test/TestOptimization.kt
View File

@ -321,4 +321,43 @@ class TestOptimization: FunSpec({
func2.statements.size shouldBe 1
(func2.statements[0] as Assignment).target.identifier!!.nameInSource shouldBe listOf("cx16", "r0")
}
test("test simple augmented assignment optimization correctly initializes all variables") {
val src="""
main {
sub start() {
ubyte z1
z1 = 10
ubyte z2
z2 = z1+z2+5
}
}"""
val result = compileText(C64Target, optimize=true, src, writeAssembly=false).assertSuccess()
/* expected:
ubyte z1
z1 = 10
ubyte z2
z2 = 0
z2 += z1 ; TODO actually add optimization to make this even better: no =0, and this should become z2 = z1
z2 += 5
*/
val statements = result.program.entrypoint.statements
statements.size shouldBe 6 // TODO 5
val z1decl = statements[0] as VarDecl
val z1init = statements[1] as Assignment
val z2decl = statements[2] as VarDecl
val z2init = statements[3] as Assignment
val z2plus1 = statements[4] as Assignment
val z2plus2= statements[5] as Assignment
z1decl.name shouldBe "z1"
z1init.value shouldBe NumericLiteralValue(DataType.UBYTE, 10.0, Position.DUMMY)
z2decl.name shouldBe "z2"
z2init.value shouldBe NumericLiteralValue(DataType.UBYTE, 0.0, Position.DUMMY)
z2plus1.isAugmentable shouldBe true
(z2plus1.value as BinaryExpression).operator shouldBe "+"
(z2plus1.value as BinaryExpression).right shouldBe IdentifierReference(listOf("z1"), Position.DUMMY)
(z2plus2.value as BinaryExpression).operator shouldBe "+"
(z2plus2.value as BinaryExpression).right shouldBe NumericLiteralValue(DataType.UBYTE, 5.0, Position.DUMMY)
}
})

6
compilerAst/src/prog8/ast/AstToplevel.kt
View File

@ -7,6 +7,7 @@ import prog8.ast.statements.*
import prog8.ast.walk.AstWalker
import prog8.ast.walk.IAstVisitor
import prog8.parser.SourceCode
import kotlin.reflect.typeOf
const val internedStringsModuleName = "prog8_interned_strings"
@ -262,6 +263,7 @@ interface Node {
}
fun replaceChildNode(node: Node, replacement: Node)
fun copy(): Node
}
@ -302,6 +304,8 @@ open class Module(final override var statements: MutableList<Statement>,
replacement.parent = this
}
override fun copy(): Node = throw NotImplementedError("no support for duplicating a Module")
override fun toString() = "Module(name=$name, pos=$position, lib=${isLibrary})"
fun accept(visitor: IAstVisitor) = visitor.visit(this)
@ -317,6 +321,8 @@ class GlobalNamespace(val modules: Iterable<Module>): Node, INameScope {
override val statements = mutableListOf<Statement>() // not used
override var parent: Node = ParentSentinel
override fun copy(): Node = throw NotImplementedError("no support for duplicating a GlobalNamespace")
override fun lookup(scopedName: List<String>): Statement? {
throw NotImplementedError("use lookup on actual ast node instead")
}

2
compilerAst/src/prog8/ast/base/Base.kt
View File

@ -184,6 +184,8 @@ object ParentSentinel : Node {
override fun replaceChildNode(node: Node, replacement: Node) {
replacement.parent = this
}
override fun copy(): Node = throw FatalAstException("should never duplicate a ParentSentinel")
}
data class Position(val file: String, val line: Int, val startCol: Int, val endCol: Int) {

22
compilerAst/src/prog8/ast/expressions/AstExpressions.kt
View File

@ -6,7 +6,6 @@ import prog8.ast.base.*
import prog8.ast.statements.*
import prog8.ast.walk.AstWalker
import prog8.ast.walk.IAstVisitor
import prog8.compilerinterface.IMemSizer
import java.util.*
import kotlin.math.round
@ -18,6 +17,7 @@ val logicalOperators = setOf("and", "or", "xor", "not")
sealed class Expression: Node {
abstract override fun copy(): Expression
abstract fun constValue(program: Program): NumericLiteralValue?
abstract fun accept(visitor: IAstVisitor)
abstract fun accept(visitor: AstWalker, parent: Node)
@ -92,6 +92,7 @@ class PrefixExpression(val operator: String, var expression: Expression, overrid
replacement.parent = this
}
override fun copy() = PrefixExpression(operator, expression.copy(), position)
override fun constValue(program: Program): NumericLiteralValue? = null
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node)= visitor.visit(this, parent)
@ -145,9 +146,8 @@ class BinaryExpression(var left: Expression, var operator: String, var right: Ex
replacement.parent = this
}
override fun toString(): String {
return "[$left $operator $right]"
}
override fun copy() = BinaryExpression(left.copy(), operator, right.copy(), position)
override fun toString() = "[$left $operator $right]"
override val isSimple = false
@ -292,7 +292,7 @@ class ArrayIndexedExpression(var arrayvar: IdentifierReference,
return "ArrayIndexed(ident=$arrayvar, arraysize=$indexer; pos=$position)"
}
fun copy() = ArrayIndexedExpression(arrayvar.copy(), indexer.copy(), position)
override fun copy() = ArrayIndexedExpression(arrayvar.copy(), indexer.copy(), position)
}
class TypecastExpression(var expression: Expression, var type: DataType, val implicit: Boolean, override val position: Position) : Expression() {
@ -311,6 +311,7 @@ class TypecastExpression(var expression: Expression, var type: DataType, val imp
replacement.parent = this
}
override fun copy() = TypecastExpression(expression.copy(), type, implicit, position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node)= visitor.visit(this, parent)
@ -346,6 +347,7 @@ data class AddressOf(var identifier: IdentifierReference, override val position:
replacement.parent = this
}
override fun copy() = AddressOf(identifier.copy(), position)
override fun constValue(program: Program): NumericLiteralValue? = null
override fun referencesIdentifier(vararg scopedName: String) = false
override fun inferType(program: Program) = InferredTypes.knownFor(DataType.UWORD)
@ -369,6 +371,7 @@ class DirectMemoryRead(var addressExpression: Expression, override val position:
replacement.parent = this
}
override fun copy() = DirectMemoryRead(addressExpression.copy(), position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node)= visitor.visit(this, parent)
@ -388,7 +391,7 @@ class NumericLiteralValue(val type: DataType, // only numerical types allowed
val number: Double = if(type==DataType.FLOAT) numbervalue else round(numbervalue)
override val isSimple = true
fun copy() = NumericLiteralValue(type, number, position)
override fun copy() = NumericLiteralValue(type, number, position)
companion object {
fun fromBoolean(bool: Boolean, position: Position) =
@ -537,6 +540,7 @@ class CharLiteral(val value: Char,
throw FatalAstException("can't replace here")
}
override fun copy() = CharLiteral(value, altEncoding, position)
override fun referencesIdentifier(vararg scopedName: String) = false
override fun constValue(program: Program): NumericLiteralValue {
val bytevalue = program.encoding.encodeString(value.toString(), altEncoding).single()
@ -566,7 +570,7 @@ class StringLiteralValue(val value: String,
}
override val isSimple = true
fun copy() = StringLiteralValue(value, altEncoding, position)
override fun copy() = StringLiteralValue(value, altEncoding, position)
override fun replaceChildNode(node: Node, replacement: Node) {
throw FatalAstException("can't replace here")
@ -598,6 +602,7 @@ class ArrayLiteralValue(val type: InferredTypes.InferredType, // inferred be
value.forEach {it.linkParents(this)}
}
override fun copy() = throw NotImplementedError("no support for duplicating a ArrayLiteralValue")
override val isSimple = true
override fun replaceChildNode(node: Node, replacement: Node) {
@ -709,6 +714,7 @@ class RangeExpr(var from: Expression,
replacement.parent = this
}
override fun copy() = RangeExpr(from.copy(), to.copy(), step.copy(), position)
override fun constValue(program: Program): NumericLiteralValue? = null
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node)= visitor.visit(this, parent)
@ -766,6 +772,7 @@ data class IdentifierReference(val nameInSource: List<String>, override val posi
throw FatalAstException("can't replace here")
}
override fun copy() = IdentifierReference(nameInSource, position)
override fun constValue(program: Program): NumericLiteralValue? {
val node = definingScope.lookup(nameInSource) ?: throw UndefinedSymbolError(this)
val vardecl = node as? VarDecl
@ -816,6 +823,7 @@ class FunctionCall(override var target: IdentifierReference,
args.forEach { it.linkParents(this) }
}
override fun copy() = throw NotImplementedError("no support for duplicating a FunctionCall")
override val isSimple = target.nameInSource.size==1 && (target.nameInSource[0] in arrayOf("msb", "lsb", "peek", "peekw"))
override fun replaceChildNode(node: Node, replacement: Node) {

52
compilerAst/src/prog8/ast/statements/AstStatements.kt
View File

@ -12,6 +12,7 @@ interface INamedStatement {
}
sealed class Statement : Node {
abstract override fun copy(): Statement
abstract fun accept(visitor: IAstVisitor)
abstract fun accept(visitor: AstWalker, parent: Node)
@ -62,6 +63,8 @@ class BuiltinFunctionStatementPlaceholder(val name: String, override val positio
override fun replaceChildNode(node: Node, replacement: Node) {
replacement.parent = this
}
override fun copy() = throw NotImplementedError("no support for duplicating a BuiltinFunctionStatementPlaceholder")
}
data class RegisterOrStatusflag(val registerOrPair: RegisterOrPair?, val statusflag: Statusflag?)
@ -73,6 +76,8 @@ class Block(override val name: String,
override val position: Position) : Statement(), INameScope {
override lateinit var parent: Node
override fun copy() = throw NotImplementedError("no support for duplicating a Block")
override fun linkParents(parent: Node) {
this.parent = parent
statements.forEach {it.linkParents(this)}
@ -104,6 +109,7 @@ data class Directive(val directive: String, val args: List<DirectiveArg>, overri
}
override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here")
override fun copy() = Directive(directive, args.map { it.copy() }, position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
}
@ -115,6 +121,7 @@ data class DirectiveArg(val str: String?, val name: String?, val int: Int?, over
this.parent = parent
}
override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here")
override fun copy() = DirectiveArg(str, name, int, position)
}
data class Label(override val name: String, override val position: Position) : Statement(), INamedStatement {
@ -125,6 +132,7 @@ data class Label(override val name: String, override val position: Position) : S
}
override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here")
override fun copy() = Label(name, position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
@ -147,6 +155,7 @@ open class Return(var value: Expression?, final override val position: Position)
replacement.parent = this
}
override fun copy() = Return(value?.copy(), position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
@ -163,6 +172,7 @@ class Break(override val position: Position) : Statement() {
}
override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here")
override fun copy() = Break(position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
}
@ -258,8 +268,8 @@ open class VarDecl(val type: VarDeclType,
throw IllegalArgumentException("attempt to get zero value for vardecl that shouldn't get it")
}
fun copy(): VarDecl {
val c = VarDecl(type, declaredDatatype, zeropage, arraysize, name, value, isArray, autogeneratedDontRemove, sharedWithAsm, position)
override fun copy(): VarDecl {
val c = VarDecl(type, declaredDatatype, zeropage, arraysize?.copy(), name, value?.copy(), isArray, autogeneratedDontRemove, sharedWithAsm, position)
c.allowInitializeWithZero = this.allowInitializeWithZero
return c
}
@ -301,7 +311,7 @@ class ArrayIndex(var indexExpr: Expression,
fun constIndex() = (indexExpr as? NumericLiteralValue)?.number?.toInt()
infix fun isSameAs(other: ArrayIndex): Boolean = indexExpr isSameAs other.indexExpr
fun copy() = ArrayIndex(indexExpr, position)
override fun copy() = ArrayIndex(indexExpr.copy(), position)
}
open class Assignment(var target: AssignTarget, var value: Expression, final override val position: Position) : Statement() {
@ -322,6 +332,7 @@ open class Assignment(var target: AssignTarget, var value: Expression, final ove
replacement.parent = this
}
override fun copy()= Assignment(target.copy(), value.copy(), position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
@ -468,7 +479,7 @@ data class AssignTarget(var identifier: IdentifierReference?,
return false
}
fun copy() = AssignTarget(identifier?.copy(), arrayindexed?.copy(), memoryAddress?.copy(), position)
override fun copy() = AssignTarget(identifier?.copy(), arrayindexed?.copy(), memoryAddress?.copy(), position)
}
class PostIncrDecr(var target: AssignTarget, val operator: String, override val position: Position) : Statement() {
@ -485,6 +496,7 @@ class PostIncrDecr(var target: AssignTarget, val operator: String, override val
replacement.parent = this
}
override fun copy() = PostIncrDecr(target.copy(), operator, position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
@ -505,6 +517,7 @@ class Jump(val address: Int?,
}
override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here")
override fun copy() = Jump(address, identifier?.copy(), generatedLabel, position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
@ -525,6 +538,8 @@ class FunctionCallStatement(override var target: IdentifierReference,
args.forEach { it.linkParents(this) }
}
override fun copy() = throw NotImplementedError("no support for duplicating a FunctionCallStatement")
override fun replaceChildNode(node: Node, replacement: Node) {
if(node===target)
target = replacement as IdentifierReference
@ -538,9 +553,7 @@ class FunctionCallStatement(override var target: IdentifierReference,
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
override fun toString(): String {
return "FunctionCallStatement(target=$target, pos=$position)"
}
override fun toString() = "FunctionCallStatement(target=$target, pos=$position)"
}
class InlineAssembly(val assembly: String, override val position: Position) : Statement() {
@ -550,6 +563,9 @@ class InlineAssembly(val assembly: String, override val position: Position) : St
this.parent = parent
}
override fun copy() = throw NotImplementedError("no support for duplicating a InlineAssembly")
override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here")
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
@ -571,6 +587,7 @@ class AnonymousScope(override var statements: MutableList<Statement>,
replacement.parent = this
}
override fun copy() = AnonymousScope(statements.map { it.copy() }.toMutableList(), position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
}
@ -583,6 +600,7 @@ class NopStatement(override val position: Position): Statement() {
}
override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here")
override fun copy() = NopStatement(position)
override fun accept(visitor: IAstVisitor) = visitor.visit(this)
override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
}
@ -636,6 +654,8 @@ class Subroutine(override val name: String,
val asmGenInfo = AsmGenInfo()
val scopedname: String by lazy { makeScopedName(name) }
override fun copy() = throw NotImplementedError("no support for duplicating a Subroutine")
override fun linkParents(parent: Node) {
this.parent = parent
parameters.forEach { it.linkParents(this) }
@ -701,6 +721,8 @@ open class SubroutineParameter(val name: String,
override fun replaceChildNode(node: Node, replacement: Node) {
throw FatalAstException("can't replace anything in a subroutineparameter node")
}
override fun copy() = SubroutineParameter(name, type, position)
}
class IfStatement(var condition: Expression,
@ -716,6 +738,8 @@ class IfStatement(var condition: Expression,
elsepart.linkParents(this)
}
override fun copy() = throw NotImplementedError("no support for duplicating a IfStatement")
override fun replaceChildNode(node: Node, replacement: Node) {
when {
node===condition -> condition = replacement as Expression
@ -743,6 +767,8 @@ class BranchStatement(var condition: BranchCondition,
elsepart.linkParents(this)
}
override fun copy() = throw NotImplementedError("no support for duplicating a BranchStatement")
override fun replaceChildNode(node: Node, replacement: Node) {
when {
node===truepart -> truepart = replacement as AnonymousScope
@ -770,6 +796,8 @@ class ForLoop(var loopVar: IdentifierReference,
body.linkParents(this)
}
override fun copy() = throw NotImplementedError("no support for duplicating a ForLoop")
override fun replaceChildNode(node: Node, replacement: Node) {
when {
node===loopVar -> loopVar = replacement as IdentifierReference
@ -801,6 +829,8 @@ class WhileLoop(var condition: Expression,
body.linkParents(this)
}
override fun copy() = throw NotImplementedError("no support for duplicating a WhileLoop")
override fun replaceChildNode(node: Node, replacement: Node) {
when {
node===condition -> condition = replacement as Expression
@ -823,6 +853,8 @@ class RepeatLoop(var iterations: Expression?, var body: AnonymousScope, override
body.linkParents(this)
}
override fun copy() = throw NotImplementedError("no support for duplicating a RepeatLoop")
override fun replaceChildNode(node: Node, replacement: Node) {
when {
node===iterations -> iterations = replacement as Expression
@ -846,6 +878,7 @@ class UntilLoop(var body: AnonymousScope,
condition.linkParents(this)
body.linkParents(this)
}
override fun copy() = throw NotImplementedError("no support for duplicating a UntilLoop")
override fun replaceChildNode(node: Node, replacement: Node) {
when {
@ -871,6 +904,8 @@ class WhenStatement(var condition: Expression,
choices.forEach { it.linkParents(this) }
}
override fun copy() = throw NotImplementedError("no support for duplicating a WhenStatement")
override fun replaceChildNode(node: Node, replacement: Node) {
if(node===condition)
condition = replacement as Expression
@ -927,6 +962,7 @@ class WhenChoice(var values: MutableList<Expression>?, // if null, th
}
}
override fun copy() = WhenChoice(values?.map{ it.copy() }?.toMutableList(), statements.copy(), position)
override fun toString(): String {
return "Choice($values at $position)"
}
@ -955,5 +991,5 @@ class DirectMemoryWrite(var addressExpression: Expression, override val position
fun accept(visitor: IAstVisitor) = visitor.visit(this)
fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)
fun copy() = DirectMemoryWrite(addressExpression, position)
override fun copy() = DirectMemoryWrite(addressExpression.copy(), position)
}

4
docs/source/todo.rst
View File

@ -4,9 +4,6 @@ TODO
For next compiler release (7.4)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
BUG: Fix C-64sound issue in petaxian (regression since 7.3, sound on c64 build works fine on older versions)
BUG: ubyte z1
ubyte z2
z2=z1+z2+5 CRASHES COMPILER
Blocked by an official Commander-x16 v39 release
@ -17,6 +14,7 @@ Blocked by an official Commander-x16 v39 release
Future
^^^^^^
- use UByte instead of Short
- simplifyConditionalExpression() should not split expression if it still results in stack-based evaluation
- remove special code generation for while and util expression
by rewriting while and until expressions into if+jump (consider them syntactic sugar)