analysis: fix the order of lookup

We need to search the scopes in reverse order. It exposed a second bug. Which is that a variable defined in the outer scope is redefined in the current scope according to the current logic in visit_Assign_one. Fix the logic to define it only if it hasn't been defined before. Fixes: py2many#417
MiguelMarcelino · Jul 12, 2021 · c7419cf · c7419cf
1 parent e350e70
commit c7419cf
Show file tree

Hide file tree

Showing 10 changed files with 30 additions and 11 deletions.
diff --git a/py2many/rewriters.py b/py2many/rewriters.py
@@ -26,7 +26,9 @@ def visit_Assign(self, node):
 
         assigns = []
         for assign_target in node.targets:
-            definition = node.scopes.find(get_id(assign_target))
+            definition = node.scopes.parent_scopes.find(get_id(assign_target))
+            if definition is None:
+                definition = node.scopes.find(get_id(assign_target))
             if definition is not assign_target:
                 previous_type = get_inferred_type(definition)
                 if get_id(previous_type) == get_id(annotation):

diff --git a/py2many/scope.py b/py2many/scope.py
@@ -52,7 +52,7 @@ def find_definition(scope, var_attr="vars"):
                 if get_id(var) == lookup:
                     return var
 
-        for scope in self:
+        for scope in reversed(self):
             defn = None
             if not defn and hasattr(scope, "vars"):
                 defn = find_definition(scope, "vars")
@@ -72,6 +72,10 @@ def find_import(self, lookup):
                     if imp.name == lookup:
                         return imp
 
+    @property
+    def parent_scopes(self):
+        return ScopeList(self[:-1])
+
 
 class ScopeTransformer(ast.NodeTransformer, ScopeMixin):
     """

diff --git a/pycpp/transpiler.py b/pycpp/transpiler.py
@@ -608,8 +608,9 @@ def _visit_AssignOne(self, node, target):
             value = self.visit(node.value)
             return "{0} = {1};".format(target, value)
 
-        target_id = get_id(target)
-        definition = node.scopes.find(target_id)
+        definition = node.scopes.parent_scopes.find(get_id(target))
+        if definition is None:
+            definition = node.scopes.find(get_id(target))
         if isinstance(target, ast.Name) and defined_before(definition, node):
             target = self.visit(target)
             value = self.visit(node.value)

diff --git a/pydart/transpiler.py b/pydart/transpiler.py
@@ -513,7 +513,9 @@ def _visit_AssignOne(self, node, target):
             value = self.visit(node.value)
             return f"{target} = {value};"
 
-        definition = node.scopes.find(get_id(target))
+        definition = node.scopes.parent_scopes.find(get_id(target))
+        if definition is None:
+            definition = node.scopes.find(get_id(target))
         if isinstance(target, ast.Name) and defined_before(definition, node):
             target = self.visit(target)
             value = self.visit(node.value)

diff --git a/pygo/transpiler.py b/pygo/transpiler.py
@@ -644,7 +644,9 @@ def _visit_AssignOne(self, node, target):
                 value, typename, left_annotation, right_annotation
             )
 
-        definition = node.scopes.find(get_id(target))
+        definition = node.scopes.parent_scopes.find(get_id(target))
+        if definition is None:
+            definition = node.scopes.find(get_id(target))
         if isinstance(target, ast.Name) and defined_before(definition, node):
             return f"{target_str} = {value}"
         else:

diff --git a/pyjl/transpiler.py b/pyjl/transpiler.py
@@ -519,7 +519,9 @@ def _visit_AssignOne(self, node, target):
                 value = "None"
             return "{0} = {1}".format(target, value)
 
-        definition = node.scopes.find(get_id(target))
+        definition = node.scopes.parent_scopes.find(get_id(target))
+        if definition is None:
+            definition = node.scopes.find(get_id(target))
         if isinstance(target, ast.Name) and defined_before(definition, node):
             target_str = self.visit(target)
             value = self.visit(node.value)

diff --git a/pykt/transpiler.py b/pykt/transpiler.py
@@ -497,7 +497,9 @@ def _visit_AssignOne(self, node, target):
             value = self.visit(node.value)
             return f"{target} = {value}"
 
-        definition = node.scopes.find(get_id(target))
+        definition = node.scopes.parent_scopes.find(get_id(target))
+        if definition is None:
+            definition = node.scopes.find(get_id(target))
         if isinstance(target, ast.Name) and defined_before(definition, node):
             target = self.visit(target)
             value = self.visit(node.value)

diff --git a/pynim/transpiler.py b/pynim/transpiler.py
@@ -494,7 +494,9 @@ def _visit_AssignOne(self, node, target):
             value = self.visit(node.value)
             return f"{target} = {value}"
 
-        definition = node.scopes.find(get_id(target))
+        definition = node.scopes.parent_scopes.find(get_id(target))
+        if definition is None:
+            definition = node.scopes.find(get_id(target))
         if isinstance(target, ast.Name) and defined_before(definition, node):
             target = self.visit(target)
             value = self.visit(node.value)

diff --git a/pyrs/transpiler.py b/pyrs/transpiler.py
@@ -814,7 +814,9 @@ def _visit_AssignOne(self, node, target):
                 value = "None"
             return "{0} = {1};".format(target, value)
 
-        definition = node.scopes.find(get_id(target))
+        definition = node.scopes.parent_scopes.find(get_id(target))
+        if definition is None:
+            definition = node.scopes.find(get_id(target))
         if isinstance(target, ast.Name) and defined_before(definition, node):
             needs_cast = self._needs_cast(target, node.value)
             target_str = self.visit(target)

diff --git a/tests/expected/comb_sort.py b/tests/expected/comb_sort.py
@@ -15,7 +15,7 @@ def comb_sort(seq: List[int]) -> List[int]:
         for i in range(len(seq) - gap):
             if seq[i] > seq[i + gap]:
                 if True:
-                    (__tmp1, __tmp2) = (seq[i + gap], seq[i])
+                    __tmp1, __tmp2 = seq[i + gap], seq[i]
                     seq[i] = __tmp1
                     seq[i + gap] = __tmp2
                 swap = True