[GR-23256] Make test_scope pass

ppisl · ppisl · commit 6d4a0de13e3c · 2020-08-19T17:44:37.000+02:00
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_global_statement.py b/graalpython/com.oracle.graal.python.test/src/tests/test_global_statement.py
@@ -0,0 +1,121 @@
+# Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+#
+# The Universal Permissive License (UPL), Version 1.0
+#
+# Subject to the condition set forth below, permission is hereby granted to any
+# person obtaining a copy of this software, associated documentation and/or
+# data (collectively the "Software"), free of charge and under any and all
+# copyright rights in the Software, and any and all patent rights owned or
+# freely licensable by each licensor hereunder covering either (i) the
+# unmodified Software as contributed to or provided by such licensor, or (ii)
+# the Larger Works (as defined below), to deal in both
+#
+# (a) the Software, and
+#
+# (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+# one is included with the Software each a "Larger Work" to which the Software
+# is contributed by such licensors),
+#
+# without restriction, including without limitation the rights to copy, create
+# derivative works of, display, perform, and distribute the Software and make,
+# use, sell, offer for sale, import, export, have made, and have sold the
+# Software and the Larger Work(s), and to sublicense the foregoing rights on
+# either these or other terms.
+#
+# This license is subject to the following condition:
+#
+# The above copyright notice and either this complete permission notice or at a
+# minimum a reference to the UPL must be included in all copies or substantial
+# portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+import unittest
+from test.support import run_unittest
+
+class BasicTests(unittest.TestCase):
+
+    def test_in_local(self):
+        loc = {}
+        glob = {}
+        exec("""if 1:
+            x = 3
+            def f():
+              x
+        """, glob, loc)
+        # normally the variables are placed in the local
+        self.assertEqual(loc['x'], 3)
+
+    def test_put_to_global(self):
+        loc = {}
+        glob = {}
+        exec("""if 1:
+            x = 3
+            def f():
+              global x
+        """, glob, loc)
+        # if a variable is marked as global, the variable is not in the local, but in global directory
+        self.assertEqual(glob['x'], 3)
+
+    def test_global_in_inner_scope(self):
+        loc = {}
+        glob = {'self': self}
+        exec("""if 1:
+            x = 4
+            def f():
+              self.assertEqual(x, 4)
+              def g():
+                global x
+            f()
+        """, glob, loc)
+        self.assertEqual(glob['x'], 4)
+
+    def test_get_global_in_inner_scope(self):
+        loc = {}
+        glob = {'self': self}
+        # if some scope declare a variable as global, then the inner scopes read
+        # it as global as well. 
+        exec("""if 1:
+            x = 5
+            def f():
+              x = 1
+              self.assertEqual(x, 1)
+              def g():
+                global x
+                def h():
+                  self.assertEqual(x, 5)
+                h()
+              g()
+            f()
+        """, glob, loc)
+        self.assertEqual(glob['x'], 5)
+
+    def test_set_local_in_inner_scope(self):
+        loc = {}
+        glob = {'self': self}
+        exec("""if 1:
+            x = 6
+            def f():
+              x = 1
+              self.assertEqual(x, 1)
+              def g():
+                global x
+                def h():
+                  x = 11
+                  self.assertEqual(x, 11)
+                h()
+              g()
+            f()
+        """, glob, loc)
+        self.assertEqual(glob['x'], 6)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_scope.txt b/graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_scope.txt
@@ -9,8 +9,10 @@
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testExtraNesting
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testFreeVarInMethod
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testFreeingCell
+*graalpython.lib-python.3.test.test_scope.ScopeTests.testGlobalInParallelNestedFunctions
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testInteractionWithTraceFunc
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testLambdas
+*graalpython.lib-python.3.test.test_scope.ScopeTests.testLeaks
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testListCompLocalVars
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testLocalsClass
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testLocalsClass_WithTrace
@@ -26,6 +28,7 @@
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testNonLocalGenerator
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testNonLocalMethod
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testRecursion
+*graalpython.lib-python.3.test.test_scope.ScopeTests.testScopeOfGlobalStmt
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testSimpleAndRebinding
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testSimpleNesting
 *graalpython.lib-python.3.test.test_scope.ScopeTests.testTopIsNotSignificant
diff --git a/graalpython/com.oracle.graal.python.test/testData/goldenFiles/BasicTests/global02.tast b/graalpython/com.oracle.graal.python.test/testData/goldenFiles/BasicTests/global02.tast
@@ -2,7 +2,7 @@ ModuleRootNode Name: <module 'global02'> SourceSection: [0,8]`global x`
   Signature: varArgs=False, varKeywordArgs=False, noArguments=True, positionalOnly=True, requiresKeywordArgs=False
   FreeVars: None
   NeedsCellFrame: False
-  FrameDescriptor: 1 slots [x]
+  FrameDescriptor:  Empty
   Documentation: None
   InnerRootNode SourceSection: [0,8]`global x`
     EmptyNode SourceSection: [0,8]`global x`
diff --git a/graalpython/com.oracle.graal.python.test/testData/goldenFiles/FuncDefTests/functionDef15.scope b/graalpython/com.oracle.graal.python.test/testData/goldenFiles/FuncDefTests/functionDef15.scope
@@ -1,6 +1,6 @@
 Scope: []
     Kind: Module
-    FrameDescriptor: [SOMETHING]
+    FrameDescriptor: Empty
     CellVars: Empty
     FreeVars: Empty
     Scope: install
diff --git a/graalpython/com.oracle.graal.python.test/testData/goldenFiles/FuncDefTests/functionDef15.tast b/graalpython/com.oracle.graal.python.test/testData/goldenFiles/FuncDefTests/functionDef15.tast
@@ -2,12 +2,14 @@ ModuleRootNode Name: <module 'functionDef15'> SourceSection: [0,114]`SOMETHING =
   Signature: varArgs=False, varKeywordArgs=False, noArguments=True, positionalOnly=True, requiresKeywordArgs=False
   FreeVars: None
   NeedsCellFrame: False
-  FrameDescriptor: 1 slots [SOMETHING]
+  FrameDescriptor:  Empty
   Documentation: None
   InnerRootNode SourceSection: [0,114]`SOMETHING = NONE↵def...`
     ExpressionWithSideEffects SourceSection: [0,114]`SOMETHING = NONE↵def...`
-      WriteNameNodeGen SourceSection: [0,16]`SOMETHING = NONE`
+      WriteGlobalNodeGen SourceSection: [0,16]`SOMETHING = NONE`
         Identifier: SOMETHING
+        IsBuiltinClassProfile SourceSection: None
+          CachedDispatchFirst SourceSection: None
         ReadNameNodeGen SourceSection: [12,16]`NONE`
           Identifier: NONE
           IsBuiltinClassProfile SourceSection: None
diff --git a/graalpython/com.oracle.graal.python.test/testData/testFiles/BasicTests/global01.scope b/graalpython/com.oracle.graal.python.test/testData/testFiles/BasicTests/global01.scope
@@ -1,6 +1,6 @@
 Scope: []
     Kind: Module
-    FrameDescriptor: [c]
+    FrameDescriptor: Empty
     CellVars: Empty
     FreeVars: Empty
     Scope: add
diff --git a/graalpython/com.oracle.graal.python.test/testData/testFiles/BasicTests/global01.tast b/graalpython/com.oracle.graal.python.test/testData/testFiles/BasicTests/global01.tast
@@ -2,12 +2,14 @@ ModuleRootNode Name: <module 'global01'> SourceSection: [0,137]`c = 0 # global v
   Signature: varArgs=False, varKeywordArgs=False, noArguments=True, positionalOnly=True, requiresKeywordArgs=False
   FreeVars: None
   NeedsCellFrame: False
-  FrameDescriptor: 1 slots [c]
+  FrameDescriptor:  Empty
   Documentation: None
   InnerRootNode SourceSection: [0,137]`c = 0 # global varia...`
     ExpressionWithSideEffects SourceSection: [0,137]`c = 0 # global varia...`
-      WriteNameNodeGen SourceSection: [0,5]`c = 0`
+      WriteGlobalNodeGen SourceSection: [0,5]`c = 0`
         Identifier: c
+        IsBuiltinClassProfile SourceSection: None
+          CachedDispatchFirst SourceSection: None
         IntegerLiteralNode SourceSection: [4,5]`0`
           Value: 0
       WriteNameNodeGen SourceSection: [25,111]`def add():↵    globa...`
@@ -76,7 +78,6 @@ ModuleRootNode Name: <module 'global01'> SourceSection: [0,137]`c = 0 # global v
           IsBuiltinClassProfile SourceSection: None
             CachedDispatchFirst SourceSection: None
         StringLiteralNode SourceSection: [123,133]`"In main:"`
-        ReadNameNodeGen SourceSection: [135,136]`c`
+        ReadGlobalOrBuiltinNodeGen SourceSection: [135,136]`c`
           Identifier: c
-          IsBuiltinClassProfile SourceSection: None
-            CachedDispatchFirst SourceSection: None
+          ReadAttributeFromObjectNotTypeNodeGen SourceSection: None
diff --git a/graalpython/com.oracle.graal.python.test/testData/testFiles/RuntimeFileTests/site.scope b/graalpython/com.oracle.graal.python.test/testData/testFiles/RuntimeFileTests/site.scope
@@ -1,6 +1,6 @@
 Scope: []
     Kind: Module
-    FrameDescriptor: [ENABLE_USER_SITE, PREFIXES, USER_BASE, USER_SITE]
+    FrameDescriptor: Empty
     CellVars: Empty
     FreeVars: Empty
     Scope: _script
diff --git a/graalpython/com.oracle.graal.python.test/testData/testFiles/RuntimeFileTests/site.tast b/graalpython/com.oracle.graal.python.test/testData/testFiles/RuntimeFileTests/site.tast
@@ -2,7 +2,7 @@ ModuleRootNode Name: <module 'site'> SourceSection: [0,21269]`"""Append module s
   Signature: varArgs=False, varKeywordArgs=False, noArguments=True, positionalOnly=True, requiresKeywordArgs=False
   FreeVars: None
   NeedsCellFrame: False
-  FrameDescriptor: 4 slots [USER_BASE, USER_SITE, PREFIXES, ENABLE_USER_SITE]
+  FrameDescriptor:  Empty
   Documentation: `Append module search...`
   InnerRootNode SourceSection: [0,21269]`"""Append module sea...`
     ExpressionWithSideEffects SourceSection: [0,21269]`"""Append module sea...`
@@ -28,8 +28,10 @@ ModuleRootNode Name: <module 'site'> SourceSection: [0,21269]`"""Append module s
         ImportExpression SourceSection: None
           ImportNode SourceSection: None
             Module: _sitebuiltins
-      WriteNameNodeGen SourceSection: [3140,3180]`PREFIXES = [sys.pref...`
+      WriteGlobalNodeGen SourceSection: [3140,3180]`PREFIXES = [sys.pref...`
         Identifier: PREFIXES
+        IsBuiltinClassProfile SourceSection: None
+          CachedDispatchFirst SourceSection: None
         ListLiteralNode SourceSection: [3151,3180]`[sys.prefix, sys.exe...`
           GetAttributeNodeGen SourceSection: [3152,3162]`sys.prefix`
             GetFixedAttributeNodeGen SourceSection: None
@@ -56,14 +58,20 @@ ModuleRootNode Name: <module 'site'> SourceSection: [0,21269]`"""Append module s
               IsBuiltinClassProfile SourceSection: None
                 CachedDispatchFirst SourceSection: None
           PythonObjectFactoryNodeGen SourceSection: None
-      WriteNameNodeGen SourceSection: [3293,3316]`ENABLE_USER_SITE = N...`
+      WriteGlobalNodeGen SourceSection: [3293,3316]`ENABLE_USER_SITE = N...`
         Identifier: ENABLE_USER_SITE
+        IsBuiltinClassProfile SourceSection: None
+          CachedDispatchFirst SourceSection: None
         ObjectLiteralNode SourceSection: [3312,3316]`None`
-      WriteNameNodeGen SourceSection: [3490,3506]`USER_SITE = None`
+      WriteGlobalNodeGen SourceSection: [3490,3506]`USER_SITE = None`
         Identifier: USER_SITE
+        IsBuiltinClassProfile SourceSection: None
+          CachedDispatchFirst SourceSection: None
         ObjectLiteralNode SourceSection: [3502,3506]`None`
-      WriteNameNodeGen SourceSection: [3507,3523]`USER_BASE = None`
+      WriteGlobalNodeGen SourceSection: [3507,3523]`USER_BASE = None`
         Identifier: USER_BASE
+        IsBuiltinClassProfile SourceSection: None
+          CachedDispatchFirst SourceSection: None
         ObjectLiteralNode SourceSection: [3519,3523]`None`
       WriteNameNodeGen SourceSection: [3526,3696]`def makepath(*paths)...`
         Identifier: makepath
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonSSTNodeFactory.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonSSTNodeFactory.java
@@ -161,7 +161,8 @@ public SSTNode registerGlobal(String[] names, int startOffset, int endOffset) {
                 throw errors.raiseInvalidSyntax(source, createSourceSection(startOffset, endOffset), ErrorMessages.NAME_IS_ASSIGNED_BEFORE_GLOBAL, name);
             }
             scopeInfo.addExplicitGlobalVariable(name);
-            globalScope.createSlotIfNotPresent(name);
+            // place the global variable into global space, see test_global_statemnt.py
+            globalScope.addExplicitGlobalVariable(name);
         }
         return new SimpleSSTNode(SimpleSSTNode.Type.EMPTY, startOffset, endOffset);
     }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/ScopeEnvironment.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/ScopeEnvironment.java
@@ -190,6 +190,7 @@ public ScopeInfo popScope() {
             // clean the variables
             localySeenVars.clear();
         }
+
         if (definingScopeKind == ScopeInfo.ScopeKind.Class) {
             boolean copy = false;
             for (Object identifier : identifiers) {
@@ -205,6 +206,31 @@ public ScopeInfo popScope() {
                 // we copy it because the indexes are now wrong due the issue GR-17984
                 definingScope.setFrameDescriptor(definingScope.getFrameDescriptor().copy());
             }
+        } else {
+            if (definingScope.hasExplicitGlobalVariables()) {
+                Set<String> globalsVariables = definingScope.getExplicitGlobalVariables();
+                List<ScopeInfo> usedInScopes;
+                for (String varName : globalsVariables) {
+                    usedInScopes = unresolvedVars.get(varName);
+                    // In inner scopes, where a variable is just read and is marked as global in
+                    // some outer scope, has to be read as global as well. But if in the inner scope
+                    // it is written into to variabl, then it's local one. See
+                    // test_global_statement.py.
+                    if (usedInScopes != null) {
+                        List<ScopeInfo> copy = new ArrayList<>(usedInScopes);
+                        for (ScopeInfo scope : copy) {
+                            ScopeInfo tmpScope = scope;
+                            ScopeInfo parentDefiningScope = definingScope.getParent();
+                            while (tmpScope != null && tmpScope != definingScope && tmpScope != parentDefiningScope) {
+                                tmpScope = tmpScope.getParent();
+                            }
+                            if (definingScope == tmpScope) {
+                                usedInScopes.remove(scope);
+                            }
+                        }
+                    }
+                }
+            }
         }
         currentScope = currentScope.getParent();
         return definingScope;
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/ScopeInfo.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/ScopeInfo.java
@@ -218,6 +218,14 @@ public boolean isExplicitGlobalVariable(String identifier) {
         return explicitGlobalVariables != null && explicitGlobalVariables.contains(identifier);
     }
 
+    public boolean hasExplicitGlobalVariables() {
+        return explicitGlobalVariables != null && !explicitGlobalVariables.isEmpty();
+    }
+
+    public Set<String> getExplicitGlobalVariables() {
+        return explicitGlobalVariables;
+    }
+
     public boolean isExplicitNonlocalVariable(String identifier) {
         return explicitNonlocalVariables != null && explicitNonlocalVariables.contains(identifier);
     }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/sst/SerializationUtils.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/sst/SerializationUtils.java
@@ -45,7 +45,7 @@
 import com.oracle.graal.python.nodes.expression.UnaryArithmetic;
 
 public class SerializationUtils {
-    public static byte VERSION = 5;
+    public static byte VERSION = 6;
 
     public static enum SSTId {
         AndID,
diff --git a/graalpython/lib-python/3/test/test_scope.py b/graalpython/lib-python/3/test/test_scope.py
@@ -1,6 +1,7 @@
 import unittest
 import weakref
 
+from test import support
 from test.support import check_syntax_error, cpython_only
 
 
@@ -402,6 +403,7 @@ def get(self):
             self.assertEqual(g.get(), 13)
             """)
 
+    @support.impl_detail("finalization", graalvm=False)
     def testLeaks(self):
 
         class Foo:

Original file line number	Diff line number	Diff line change
`@@ -161,7 +161,8 @@ public SSTNode registerGlobal(String[] names, int startOffset, int endOffset) {`
`161`	`161`	`throw errors.raiseInvalidSyntax(source, createSourceSection(startOffset, endOffset), ErrorMessages.NAME_IS_ASSIGNED_BEFORE_GLOBAL, name);`
`162`	`162`	`}`
`163`	`163`	`scopeInfo.addExplicitGlobalVariable(name);`
`164`		`- globalScope.createSlotIfNotPresent(name);`
	`164`	`+ // place the global variable into global space, see test_global_statemnt.py`
	`165`	`+ globalScope.addExplicitGlobalVariable(name);`
`165`	`166`	`}`
`166`	`167`	`return new SimpleSSTNode(SimpleSSTNode.Type.EMPTY, startOffset, endOffset);`
`167`	`168`	`}`