Merge pull request #360 from boriel/bugfix/optimization_wrong

Ensure function calls on some optimizations cases

Author: boriel

api/optimize.py

@@ -148,7 +148,7 @@ def visit_FUNCTION(self, node):
     def visit_LET(self, node):
         if self.O_LEVEL > 1 and not node.children[0].accessed:
             warning_not_used(node.children[0].lineno, node.children[0].name)
-            yield self.NOP
+            yield symbols.BLOCK(*list(self.filter_inorder(node.children[1], lambda x: isinstance(x, symbols.CALL))))
         else:
             yield (yield self.generic_visit(node))
 

ast_/ast.py

@@ -9,6 +9,7 @@
 #                    the GNU General License
 # ----------------------------------------------------------------------
 
+from typing import Callable, Any
 import types
 from .tree import Tree
 
@@ -50,5 +51,16 @@ def _visit(self, node):
         return meth(node)
 
     @staticmethod
-    def generic_visit(node):
+    def generic_visit(node: Ast):
         raise RuntimeError("No {}() method defined".format('visit_' + node.token))
+
+    def filter_inorder(self, node, filter_func: Callable[[Any], bool]):
+        """ Visit the tree inorder, but only those that return true for filter
+        """
+        stack = [node]
+        while stack:
+            node = stack.pop()
+            if filter_func(node):
+                yield self.visit(node)
+            elif isinstance(node, Ast):
+                stack.extend(node.children[::-1])

ast_/tree.py

@@ -29,22 +29,22 @@ class Tree:
     class ChildrenList:
         def __init__(self, node: 'Tree'):
             assert isinstance(node, Tree)
-            self.node = node  # Node having this children
+            self.parent = node  # Node having this children
             self._children = []
 
         def __getitem__(self, key):
             if isinstance(key, int):
                 return self._children[key]
 
-            result = Tree.ChildrenList(self.node)
+            result = Tree.ChildrenList(self.parent)
             for x in self._children[key]:
                 result.append(x)
             return result
 
         def __setitem__(self, key, value):
             assert value is None or isinstance(value, Tree)
             if value is not None:
-                value.parent = self.node
+                value.parent = self.parent
             self._children[key] = value
 
         def __delitem__(self, key):
@@ -53,12 +53,12 @@ def __delitem__(self, key):
 
         def append(self, value):
             assert isinstance(value, Tree)
-            value.parent = self.node
+            value.parent = self.parent
             self._children.append(value)
 
         def insert(self, pos, value):
             assert isinstance(value, Tree)
-            value.parent = self.node
+            value.parent = self.parent
             self._children.insert(pos, value)
 
         def pop(self, pos=-1):
@@ -73,15 +73,15 @@ def __add__(self, other):
             if not isinstance(other, Tree.ChildrenList):
                 assert isinstance(other, collections.Container)
 
-            result = Tree.ChildrenList(self.node)
+            result = Tree.ChildrenList(self.parent)
             for x in self:
                 result.append(x)
             for x in other:
                 result.append(x)
             return result
 
         def __repr__(self):
-            return "%s:%s" % (self.node.__repr__(), str([x.__repr__() for x in self._children]))
+            return "%s:%s" % (self.parent.__repr__(), str([x.__repr__() for x in self._children]))
 
     def __init__(self):
         self._children = Tree.ChildrenList(self)

tests/functional/opt2_unused_var.asm

@@ -0,0 +1,85 @@
+	org 32768
+__START_PROGRAM:
+	di
+	push ix
+	push iy
+	exx
+	push hl
+	exx
+	ld hl, 0
+	add hl, sp
+	ld (__CALL_BACK__), hl
+	ei
+	call _Check
+	ld hl, 0
+	call __PAUSE
+	ld hl, 0
+	ld b, h
+	ld c, l
+__END_PROGRAM:
+	di
+	ld hl, (__CALL_BACK__)
+	ld sp, hl
+	exx
+	pop hl
+	pop iy
+	pop ix
+	exx
+	ei
+	ret
+__CALL_BACK__:
+	DEFW 0
+_TestPrint:
+	push ix
+	ld ix, 0
+	add ix, sp
+	ld a, (ix+7)
+	ld (0), a
+	ld a, (ix+5)
+	ld (0), a
+_TestPrint__leave:
+	ld sp, ix
+	pop ix
+	exx
+	pop hl
+	pop bc
+	ex (sp), hl
+	exx
+	ret
+_Check:
+	push ix
+	ld ix, 0
+	add ix, sp
+	ld a, 10
+	push af
+	ld a, 5
+	push af
+	call _TestPrint
+	push bc
+	push de
+	push af
+	ld a, 11
+	push af
+	ld a, 6
+	push af
+	call _TestPrint
+	push bc
+	push de
+	push af
+_Check__leave:
+	ld sp, ix
+	pop ix
+	ret
+#line 1 "pause.asm"
+	; The PAUSE statement (Calling the ROM)
+__PAUSE:
+		ld b, h
+	    ld c, l
+	    jp 1F3Dh  ; PAUSE_1
+#line 62 "opt2_unused_var.bas"
+ZXBASIC_USER_DATA:
+; Defines DATA END --> HEAP size is 0
+ZXBASIC_USER_DATA_END:
+	; Defines USER DATA Length in bytes
+ZXBASIC_USER_DATA_LEN EQU ZXBASIC_USER_DATA_END - ZXBASIC_USER_DATA
+	END

tests/functional/opt2_unused_var.bas

@@ -0,0 +1,16 @@
+' BugTest
+Check()
+pause 0
+
+
+function TestPrint(y as ubyte, x as ubyte)
+  poke 0, x
+  poke 0, y
+end function
+
+
+SUB Check()
+  dim y as ubyte  
+  y=TestPrint(5,10) * 5 + (4 + TestPrint(6, 11))
+end sub
+