From 299419917ec86149ca3f324ceef388081e9e331b Mon Sep 17 00:00:00 2001
From: Irmen de Jong <irmen@razorvine.net>
Date: Tue, 19 Dec 2023 23:44:22 +0100
Subject: [PATCH] added symbol ambiguity error (variable vs block name for
 scoped symbols)

fixes #114
---
 .../compiler/astprocessing/AstChecker.kt      | 12 +++++++--
 .../astprocessing/AstIdentifiersChecker.kt    | 14 ++++++-----
 .../test/codegeneration/TestVariousCodeGen.kt | 25 ++++++++++++++++---
 docs/source/todo.rst                          |  2 +-
 examples/test.p8                              |  4 +--
 5 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt
index adb21da3f..39b2d4251 100644
--- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt
+++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt
@@ -75,6 +75,13 @@ internal class AstChecker(private val program: Program,
                 }
             }
         }
+
+        if(identifier.nameInSource.size>1) {
+            val lookupModule = identifier.definingScope.lookup(identifier.nameInSource.take(1))
+            if(lookupModule is VarDecl) {
+                errors.err("ambiguous symbol name, block name expected but found variable", identifier.position)
+            }
+        }
     }
 
     override fun visit(unrollLoop: UnrollLoop) {
@@ -1340,9 +1347,10 @@ internal class AstChecker(private val program: Program,
                 }
             }
         } else if(postIncrDecr.target.arrayindexed != null) {
-            val target = postIncrDecr.target.arrayindexed?.arrayvar?.targetStatement(program)
+            val indexed = postIncrDecr.target.arrayindexed!!
+            val target = indexed.arrayvar.targetStatement(program)
             if(target==null) {
-                errors.err("undefined symbol", postIncrDecr.position)
+                errors.undefined(indexed.arrayvar.nameInSource, indexed.arrayvar.position)
             }
             else {
                 val dt = (target as VarDecl).datatype
diff --git a/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt
index 4c02b9344..6d7840e8d 100644
--- a/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt
+++ b/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt
@@ -24,7 +24,7 @@ internal class AstIdentifiersChecker(private val errors: IErrorReporter,
     }
 
     private fun nameShadowWarning(name: String, position: Position, existing: Statement) {
-        errors.warn("name '$name' shadows occurrence at ${existing.position.file} line ${existing.position.line}", position)
+        errors.warn("name '$name' shadows the definition at ${existing.position.file} line ${existing.position.line}", position)
     }
 
     override fun visit(block: Block) {
@@ -56,11 +56,13 @@ internal class AstIdentifiersChecker(private val errors: IErrorReporter,
                 nameError(decl.name, decl.position, existingInSameScope)
 
             val existingOuter = decl.parent.definingScope.lookup(listOf(decl.name))
-            if (existingOuter != null && existingOuter !== decl && existingOuter is VarDecl) {
-                if (existingOuter.parent !== decl.parent)
-                    nameShadowWarning(decl.name, decl.position, existingOuter)
-                else
-                    nameError(decl.name, decl.position, existingOuter)
+            if (existingOuter != null && existingOuter !== decl) {
+                if (existingOuter is VarDecl) {
+                    if (existingOuter.parent !== decl.parent)
+                        nameShadowWarning(decl.name, decl.position, existingOuter)
+                    else
+                        nameError(decl.name, decl.position, existingOuter)
+                }
             }
         }
 
diff --git a/compiler/test/codegeneration/TestVariousCodeGen.kt b/compiler/test/codegeneration/TestVariousCodeGen.kt
index 50ea92571..3d1949a77 100644
--- a/compiler/test/codegeneration/TestVariousCodeGen.kt
+++ b/compiler/test/codegeneration/TestVariousCodeGen.kt
@@ -7,7 +7,6 @@ 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
 import prog8.code.ast.PtAssignment
 import prog8.code.ast.PtBinaryExpression
 import prog8.code.ast.PtVariable
@@ -283,8 +282,28 @@ derp {
     }
 }"""
 
-        compileText(VMTarget(), true, src, writeAssembly = true) shouldNotBe null
-        compileText(Cx16Target(), true, src, writeAssembly = true) shouldNotBe null
+        compileText(VMTarget(), false, src, writeAssembly = true) shouldNotBe null
+        compileText(Cx16Target(), false, src, writeAssembly = true) shouldNotBe null
+    }
+
+    test("ambiguous symbol name variable vs block") {
+        val src = """
+main {
+    sub start() {
+        uword module
+        module++
+        module.test++
+    }
+}
+
+module {
+    ubyte @shared test
+}
+"""
+        val errors=ErrorReporterForTests()
+        compileText(VMTarget(), false, src, writeAssembly = false, errors = errors) shouldBe null
+        errors.errors.size shouldBe 1
+        errors.errors[0] shouldContain "ambiguous symbol"
     }
 
     test("prefix expressions with typecasting") {
diff --git a/docs/source/todo.rst b/docs/source/todo.rst
index 58ffdf2ca..19b882de1 100644
--- a/docs/source/todo.rst
+++ b/docs/source/todo.rst
@@ -2,7 +2,7 @@
 TODO
 ====
 
-- fix the 'message' github compiler error (github)
+- also change asm subroutine and variable prefixes from `p8_` to `p8s_` / `p8v_` and be done with it once and for all.
 
 
 - verafx vram-vram copy routine?
diff --git a/examples/test.p8 b/examples/test.p8
index dbb590d7e..5f2f424e7 100644
--- a/examples/test.p8
+++ b/examples/test.p8
@@ -3,9 +3,9 @@
 
 main {
     sub start() {
-        uword module        ; TODO shadow warning
+        uword module
         module++
-        module.test++       ; TODO compiler error
+        module.test++
     }
 }