From a170506356e462d0a7fc257737c4a7ea4ad347b5 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 27 Jan 2022 23:32:55 +0100 Subject: [PATCH] simplify IdentifierReference equality check back to default (name+pos) --- compiler/test/TestCallgraph.kt | 26 +++++++ compiler/test/TestOptimization.kt | 14 ++-- compiler/test/ast/TestIdentifierRef.kt | 70 +++++++++++++++++++ .../prog8/ast/expressions/AstExpressions.kt | 4 -- .../src/prog8/compilerinterface/CallGraph.kt | 10 +-- docs/source/todo.rst | 1 - 6 files changed, 108 insertions(+), 17 deletions(-) create mode 100644 compiler/test/ast/TestIdentifierRef.kt diff --git a/compiler/test/TestCallgraph.kt b/compiler/test/TestCallgraph.kt index fe62c1f7e..e089e6d78 100644 --- a/compiler/test/TestCallgraph.kt +++ b/compiler/test/TestCallgraph.kt @@ -2,6 +2,7 @@ package prog8tests import io.kotest.assertions.withClue import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.ints.shouldBeGreaterThanOrEqual import io.kotest.matchers.maps.shouldContainKey import io.kotest.matchers.maps.shouldNotContainKey import io.kotest.matchers.shouldBe @@ -95,4 +96,29 @@ class TestCallgraph: FunSpec({ graph.calledBy shouldNotContainKey startSub } } + + test("allIdentifiers separates for different positions of the IdentifierReferences") { + val sourcecode = """ + main { + sub start() { + uword x1 = &empty + uword x2 = &empty + empty() + } + sub empty() { + %asm {{ + nop + }} + } + } + """ + val result = compileText(C64Target, false, sourcecode).assertSuccess() + val graph = CallGraph(result.program) + graph.allIdentifiers.size shouldBeGreaterThanOrEqual 9 + val empties = graph.allIdentifiers.keys.filter { it.nameInSource==listOf("empty") } + empties.size shouldBe 3 + empties[0].position.line shouldBe 4 + empties[1].position.line shouldBe 5 + empties[2].position.line shouldBe 6 + } }) diff --git a/compiler/test/TestOptimization.kt b/compiler/test/TestOptimization.kt index cb910445d..f761d6863 100644 --- a/compiler/test/TestOptimization.kt +++ b/compiler/test/TestOptimization.kt @@ -313,16 +313,16 @@ class TestOptimization: FunSpec({ bbAssigns[0].target.identifier!!.nameInSource shouldBe listOf("bb") bbAssigns[0].value shouldBe instanceOf() (bbAssigns[0].value as PrefixExpression).operator shouldBe "not" - (bbAssigns[0].value as PrefixExpression).expression shouldBe IdentifierReference(listOf("bb"), Position.DUMMY) + ((bbAssigns[0].value as PrefixExpression).expression as? IdentifierReference)?.nameInSource shouldBe listOf("bb") bbAssigns[0].value.inferType(result.program).getOrElse { fail("dt") } shouldBe DataType.UBYTE bbAssigns[1].target.identifier!!.nameInSource shouldBe listOf("bb") val bbAssigns1expr = bbAssigns[1].value as BinaryExpression bbAssigns1expr.operator shouldBe "or" - bbAssigns1expr.left shouldBe IdentifierReference(listOf("bb"), Position.DUMMY) + (bbAssigns1expr.left as? IdentifierReference)?.nameInSource shouldBe listOf("bb") bbAssigns1expr.right shouldBe instanceOf() (bbAssigns1expr.right as PrefixExpression).operator shouldBe "not" - (bbAssigns1expr.right as PrefixExpression).expression shouldBe IdentifierReference(listOf("ww"), Position.DUMMY) + ((bbAssigns1expr.right as PrefixExpression).expression as? IdentifierReference)?.nameInSource shouldBe listOf("ww") bbAssigns1expr.inferType(result.program).getOrElse { fail("dt") } shouldBe DataType.UBYTE val asm = generateAssembly(result.program, options) @@ -380,7 +380,7 @@ class TestOptimization: FunSpec({ assignFF.target.identifier!!.nameInSource shouldBe listOf("ff") val value = assignFF.value as BinaryExpression value.operator shouldBe "+" - value.left shouldBe IdentifierReference(listOf("ff"), Position.DUMMY) + (value.left as? IdentifierReference)?.nameInSource shouldBe listOf("ff") value.right shouldBe instanceOf() val asm = generateAssembly(result.program) @@ -496,12 +496,12 @@ class TestOptimization: FunSpec({ z4decl.name shouldBe "z4" z4init.value shouldBe NumericLiteralValue(DataType.UBYTE, 0.0, Position.DUMMY) z5decl.name shouldBe "z5" - z5init.value shouldBe IdentifierReference(listOf("z1"), Position.DUMMY) + (z5init.value as? IdentifierReference)?.nameInSource shouldBe listOf("z1") z5plus.isAugmentable shouldBe true (z5plus.value as BinaryExpression).operator shouldBe "+" (z5plus.value as BinaryExpression).right shouldBe NumericLiteralValue(DataType.UBYTE, 5.0, Position.DUMMY) z6decl.name shouldBe "z6" - z6init.value shouldBe IdentifierReference(listOf("z1"), Position.DUMMY) + (z6init.value as? IdentifierReference)?.nameInSource shouldBe listOf("z1") z6plus.isAugmentable shouldBe true (z6plus.value as BinaryExpression).operator shouldBe "-" (z6plus.value as BinaryExpression).right shouldBe NumericLiteralValue(DataType.UBYTE, 5.0, Position.DUMMY) @@ -663,7 +663,7 @@ class TestOptimization: FunSpec({ assignXX2.target.identifier!!.nameInSource shouldBe listOf("xx") val xxValue = assignXX2.value as BinaryExpression xxValue.operator shouldBe "+" - xxValue.left shouldBe IdentifierReference(listOf("xx"), Position.DUMMY) + (xxValue.left as? IdentifierReference)?.nameInSource shouldBe listOf("xx") xxValue.right shouldBe NumericLiteralValue(DataType.UBYTE, 10.0, Position.DUMMY) } }) diff --git a/compiler/test/ast/TestIdentifierRef.kt b/compiler/test/ast/TestIdentifierRef.kt new file mode 100644 index 000000000..c3f48d416 --- /dev/null +++ b/compiler/test/ast/TestIdentifierRef.kt @@ -0,0 +1,70 @@ +package prog8tests.ast + +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.kotest.matchers.types.instanceOf +import prog8.ast.Program +import prog8.ast.base.Position +import prog8.ast.expressions.AddressOf +import prog8.ast.expressions.IdentifierReference +import prog8.ast.expressions.NumericLiteralValue +import prog8.ast.expressions.PrefixExpression +import prog8.ast.statements.* +import prog8.parser.Prog8Parser +import prog8.parser.SourceCode +import prog8tests.helpers.DummyFunctions +import prog8tests.helpers.DummyMemsizer +import prog8tests.helpers.DummyStringEncoder + + +class TestIdentifierRef: FunSpec({ + + test("constructor and equality") { + val ident1 = IdentifierReference(listOf("a", "b"), Position("file", 1, 2, 3)) + val ident1same = IdentifierReference(listOf("a", "b"), Position("file", 1, 2, 3)) + val ident1copy = ident1.copy() + val ident2 = IdentifierReference(listOf("a", "b", "c"), Position("file", 1, 2, 3)) + val ident3 = IdentifierReference(listOf("a", "b"), Position("file2", 11, 22, 33)) + + ident1.nameInSource shouldBe listOf("a", "b") + ident1.isSimple shouldBe true + (ident1 isSameAs ident1same) shouldBe true + (ident1 isSameAs ident1copy) shouldBe true + (ident1 isSameAs ident2) shouldBe false + (ident1 isSameAs ident3) shouldBe true // as opposed to inequality, they do refer to the same symbol! + (ident1 == ident1same) shouldBe true + (ident1 == ident1copy) shouldBe true + (ident1 == ident2) shouldBe false + (ident1 == ident3) shouldBe false + + val pfx = PrefixExpression("-", NumericLiteralValue.optimalInteger(1, Position.DUMMY), Position.DUMMY) + (ident1 isSameAs pfx) shouldBe false + } + + test("target methods") { + val src= SourceCode.Text(""" + main { + ubyte bb + uword ww + sub start() { + ww++ + ww = &main + } + } """) + val module = Prog8Parser.parseModule(src) + val program = Program("test", DummyFunctions, DummyMemsizer, DummyStringEncoder) + program.addModule(module) + val mstmts = (module.statements.single() as Block).statements + val stmts = mstmts.filterIsInstance().single().statements + val wwref = (stmts[0] as PostIncrDecr).target.identifier!! + val mainref = ((stmts[1] as Assignment).value as AddressOf).identifier + wwref.nameInSource shouldBe listOf("ww") + wwref.wasStringLiteral(program) shouldBe false + wwref.targetStatement(program) shouldBe instanceOf() + wwref.targetVarDecl(program)!!.name shouldBe "ww" + wwref.targetVarDecl(program)!!.parent shouldBe instanceOf() + mainref.nameInSource shouldBe listOf("main") + mainref.wasStringLiteral(program) shouldBe false + mainref.targetStatement(program) shouldBe instanceOf() + } +}) diff --git a/compilerAst/src/prog8/ast/expressions/AstExpressions.kt b/compilerAst/src/prog8/ast/expressions/AstExpressions.kt index 304adc24c..085137e32 100644 --- a/compilerAst/src/prog8/ast/expressions/AstExpressions.kt +++ b/compilerAst/src/prog8/ast/expressions/AstExpressions.kt @@ -861,10 +861,6 @@ data class IdentifierReference(val nameInSource: List, override val posi fun targetVarDecl(program: Program): VarDecl? = targetStatement(program) as? VarDecl fun targetSubroutine(program: Program): Subroutine? = targetStatement(program) as? Subroutine - // TODO equality also includes position, compare nameInSource explicitly if you only want name equality - override fun equals(other: Any?) = other is IdentifierReference && other.nameInSource==nameInSource - override fun hashCode() = nameInSource.hashCode() - override fun linkParents(parent: Node) { this.parent = parent } diff --git a/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt b/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt index 919fda3f2..1ede00501 100644 --- a/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt +++ b/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt @@ -16,14 +16,14 @@ class CallGraph(private val program: Program) : IAstVisitor { val importedBy = mutableMapOf>().withDefault { setOf() } val calls = mutableMapOf>().withDefault { setOf() } val calledBy = mutableMapOf>().withDefault { setOf() } - private val allIdentifiersAndTargets = mutableMapOf, Statement>() // note: identifier+location as key, because currently identifier equality is only on name + private val allIdentifiersAndTargets = mutableMapOf() private val allAssemblyNodes = mutableListOf() init { visit(program) } - val allIdentifiers: Map, Statement> = allIdentifiersAndTargets + val allIdentifiers: Map = allIdentifiersAndTargets private val usedSubroutines: Set by lazy { calledBy.keys + program.entrypoint @@ -35,7 +35,7 @@ class CallGraph(private val program: Program) : IAstVisitor { val used = mutableSetOf() allIdentifiersAndTargets.forEach { - if(it.key.first.definingBlock in blocksFromSubroutines) { + if(it.key.definingBlock in blocksFromSubroutines) { val target = it.value.definingBlock used.add(target) } @@ -115,7 +115,7 @@ class CallGraph(private val program: Program) : IAstVisitor { } override fun visit(identifier: IdentifierReference) { - allIdentifiersAndTargets[Pair(identifier, identifier.position)] = identifier.targetStatement(program)!! + allIdentifiersAndTargets[identifier] = identifier.targetStatement(program)!! } override fun visit(inlineAssembly: InlineAssembly) { @@ -219,7 +219,7 @@ class CallGraph(private val program: Program) : IAstVisitor { if(decl.definingBlock !in usedBlocks) return emptyList() - return allIdentifiersAndTargets.filter { decl===it.value }.map{ it.key.first } + return allIdentifiersAndTargets.filter { decl===it.value }.map{ it.key } } private fun nameInAssemblyCode(name: String) = diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 85620ad9f..828d080aa 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -38,7 +38,6 @@ Blocked by an official Commander-x16 r39 release Future Things and Ideas ^^^^^^^^^^^^^^^^^^^^^^^ - nameInAssemblyCode() should search smarter -- IdentifierReference: fix equality to also include position. CallGraph can then also only store IdentifierRef instead of pair(ident, position) as keys. - Fix: don't report as recursion if code assigns address of its own subroutine to something, rather than calling it - allow "xxx" * constexpr (where constexpr is not a number literal, now gives expression error not same type) - can we promise a left-to-right function call argument evaluation? without sacrificing performance