From 5da3abe6b48cc0c8cfc82c9749c0dd8e150ddbd4 Mon Sep 17 00:00:00 2001
From: Irmen de Jong <irmen@razorvine.net>
Date: Sat, 17 Jun 2023 14:44:36 +0200
Subject: [PATCH] fix silent typecast on return statements that could lose data
 (word->byte)

---
 .../compiler/astprocessing/TypecastsAdder.kt  |  4 ++-
 compiler/test/TestTypecasts.kt                | 33 ++++++++++++++++++-
 docs/source/todo.rst                          |  5 ---
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt
index 3e5f2dd2d..a31fc7ab3 100644
--- a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt
+++ b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt
@@ -380,12 +380,14 @@ class TypecastsAdder(val program: Program, val options: CompilationOptions, val
 
     override fun after(returnStmt: Return, parent: Node): Iterable<IAstModification> {
         // add a typecast to the return type if it doesn't match the subroutine's signature
+        // but only if no data loss occurs
         val returnValue = returnStmt.value
         if(returnValue!=null) {
             val subroutine = returnStmt.definingSubroutine!!
             if(subroutine.returntypes.size==1) {
                 val subReturnType = subroutine.returntypes.first()
-                if (returnValue.inferType(program) istype subReturnType)
+                val returnDt = returnValue.inferType(program)
+                if (returnDt istype subReturnType or returnDt.isNotAssignableTo(subReturnType))
                     return noModifications
                 if (returnValue is NumericLiteral) {
                     val cast = returnValue.cast(subroutine.returntypes.single())
diff --git a/compiler/test/TestTypecasts.kt b/compiler/test/TestTypecasts.kt
index 4e17b8f8c..889d2032c 100644
--- a/compiler/test/TestTypecasts.kt
+++ b/compiler/test/TestTypecasts.kt
@@ -1003,7 +1003,7 @@ main  {
     }
 
     test("byte when choices silently converted to word for convenience") {
-        var text="""
+        val text="""
 main {
   sub start() {
     uword z = 3
@@ -1016,4 +1016,35 @@ main {
 }"""
         compileText(C64Target(), false, text, writeAssembly = false) shouldNotBe null
     }
+
+    test("returning smaller dt than returndt is ok") {
+        val text="""
+main {
+    sub start() {
+        void test()
+    }
+    
+    sub test() -> uword {
+        return cx16.r0L
+    }
+}"""
+        compileText(C64Target(), false, text, writeAssembly = false) shouldNotBe null
+    }
+
+    test("returning bigger dt than returndt is not ok") {
+        val text="""
+main {
+    sub start() {
+        void test()
+    }
+    
+    sub test() -> ubyte {
+        return cx16.r0
+    }
+}"""
+        val errors=ErrorReporterForTests()
+        compileText(C64Target(), false, text, writeAssembly = false, errors=errors) shouldBe null
+        errors.errors.single() shouldContain "doesn't match"
+    }
+
 })
diff --git a/docs/source/todo.rst b/docs/source/todo.rst
index 96dce4a9d..c0b4ca968 100644
--- a/docs/source/todo.rst
+++ b/docs/source/todo.rst
@@ -1,11 +1,6 @@
 TODO
 ====
 
-- this should give a compiler error because word returnvalue:
-    sub atan_coarse_qd(ubyte quadrant, ubyte xdelta, ubyte ydelta) -> ubyte {
-        return mkword(math.atan(0, 0, xdelta, ydelta), 0) / 2730
-    }
-
 - vm: fix syscall.ATAN calculation
 - document some library modules better (diskio, etc)