simplify IdentifierReference equality check back to default (name+pos)

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
+    }
 })

compiler/test/TestOptimization.kt

@@ -313,16 +313,16 @@ class TestOptimization: FunSpec({
         bbAssigns[0].target.identifier!!.nameInSource shouldBe listOf("bb")
         bbAssigns[0].value shouldBe instanceOf<PrefixExpression>()
         (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<PrefixExpression>()
         (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<TypecastExpression>()
 
         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)
     }
 })

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<Subroutine>().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<VarDecl>()
+        wwref.targetVarDecl(program)!!.name shouldBe "ww"
+        wwref.targetVarDecl(program)!!.parent shouldBe instanceOf<Block>()
+        mainref.nameInSource shouldBe listOf("main")
+        mainref.wasStringLiteral(program) shouldBe false
+        mainref.targetStatement(program) shouldBe instanceOf<Block>()
+    }
+})

compilerAst/src/prog8/ast/expressions/AstExpressions.kt

@@ -861,10 +861,6 @@ data class IdentifierReference(val nameInSource: List<String>, 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
     }

compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt

@@ -16,14 +16,14 @@ class CallGraph(private val program: Program) : IAstVisitor {
     val importedBy = mutableMapOf<Module, Set<Module>>().withDefault { setOf() }
     val calls = mutableMapOf<Subroutine, Set<Subroutine>>().withDefault { setOf() }
     val calledBy = mutableMapOf<Subroutine, Set<Node>>().withDefault { setOf() }
-    private val allIdentifiersAndTargets = mutableMapOf<Pair<IdentifierReference, Position>, Statement>()       // note: identifier+location as key, because currently identifier equality is only on name
+    private val allIdentifiersAndTargets = mutableMapOf<IdentifierReference, Statement>()
     private val allAssemblyNodes = mutableListOf<InlineAssembly>()
 
     init {
         visit(program)
     }
 
-    val allIdentifiers: Map<Pair<IdentifierReference, Position>, Statement> = allIdentifiersAndTargets
+    val allIdentifiers: Map<IdentifierReference, Statement> = allIdentifiersAndTargets
 
     private val usedSubroutines: Set<Subroutine> by lazy {
         calledBy.keys + program.entrypoint
@@ -35,7 +35,7 @@ class CallGraph(private val program: Program) : IAstVisitor {
         val used = mutableSetOf<Block>()
 
         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) =

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