generators should take the closure from the generator function that they are created from, not cache them. since the defaults can be changed, no generator function is actually stateless

Author: timfel

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/generator/PGenerator.java

@@ -68,6 +68,8 @@ public static PGenerator create(LazyPythonClass clazz, String name, RootCallTarg
             for (int i = 0; i < closure.length; i++) {
                 generatorFrame.setObject(freeVarSlots[i], closure[i]);
             }
+        } else {
+            assert freeVarSlots.length == 0;
         }
         // initialize own cell vars to new cells (these cells will be used by nested functions to
         // create their own closures)

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/function/FunctionDefinitionNode.java

@@ -64,6 +64,7 @@ public FunctionDefinitionNode(String functionName, String enclosingClassName, Ex
         this.arity = arity;
         assert defaults == null || Arrays.stream(defaults).noneMatch(x -> x == null);
         this.defaults = defaults;
+        assert kwDefaults == null || Arrays.stream(kwDefaults).noneMatch(x -> x == null);
         this.kwDefaults = kwDefaults;
     }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/function/GeneratorFunctionDefinitionNode.java

@@ -40,6 +40,7 @@
 import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.frame.FrameDescriptor;
 import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.nodes.ExplodeLoop;
 
 public class GeneratorFunctionDefinitionNode extends FunctionDefinitionNode {
     protected final int numOfActiveFlags;
@@ -62,18 +63,14 @@ public GeneratorFunctionDefinitionNode(String name, String enclosingClassName, E
     public static GeneratorFunctionDefinitionNode create(String name, String enclosingClassName, ExpressionNode doc, Arity arity, ExpressionNode[] defaults, KwDefaultExpressionNode[] kwDefaults,
                     RootCallTarget callTarget, FrameDescriptor frameDescriptor, DefinitionCellSlots definitionCellSlots, ExecutionCellSlots executionCellSlots, int numOfActiveFlags,
                     int numOfGeneratorBlockNode, int numOfGeneratorForNode) {
-        if (defaults != null && defaults.length > 0) {
-            return new GeneratorFunctionDefinitionNode(name, enclosingClassName, doc, arity, defaults, kwDefaults, callTarget,
-                            frameDescriptor, definitionCellSlots, executionCellSlots,
-                            numOfActiveFlags, numOfGeneratorBlockNode, numOfGeneratorForNode);
-        }
-
-        return new StatelessGeneratorFunctionDefinitionNode(name, enclosingClassName, doc, arity, callTarget,
+        return new GeneratorFunctionDefinitionNode(name, enclosingClassName, doc, arity, defaults, kwDefaults, callTarget,
                         frameDescriptor, definitionCellSlots, executionCellSlots,
                         numOfActiveFlags, numOfGeneratorBlockNode, numOfGeneratorForNode);
     }
 
     @Override
+    @ExplodeLoop // this should always be safe, how many syntactic defaults can one function have...
+                 // and these are constant
     public PGeneratorFunction execute(VirtualFrame frame) {
         Object[] defaultValues = null;
         if (defaults != null) {
@@ -90,45 +87,17 @@ public PGeneratorFunction execute(VirtualFrame frame) {
             }
         }
         PCell[] closure = getClosureFromLocals(frame);
-        return withDocString(frame, factory().createGeneratorFunction(functionName, enclosingClassName, arity, getGeneratorCallTarget(closure), PArguments.getGlobals(frame), closure, defaultValues,
+        return withDocString(frame, factory().createGeneratorFunction(functionName, enclosingClassName, arity, getGeneratorCallTarget(), PArguments.getGlobals(frame), closure, defaultValues,
                         kwDefaultValues));
     }
 
-    protected RootCallTarget getGeneratorCallTarget(PCell[] closure) {
+    protected RootCallTarget getGeneratorCallTarget() {
         if (generatorCallTarget == null) {
             CompilerDirectives.transferToInterpreterAndInvalidate();
             GeneratorFunctionRootNode generatorFunctionRootNode = new GeneratorFunctionRootNode(getContext().getLanguage(), callTarget, functionName, frameDescriptor,
-                            closure, executionCellSlots, numOfActiveFlags, numOfGeneratorBlockNode, numOfGeneratorForNode);
+                            executionCellSlots, numOfActiveFlags, numOfGeneratorBlockNode, numOfGeneratorForNode);
             generatorCallTarget = Truffle.getRuntime().createCallTarget(generatorFunctionRootNode);
         }
         return generatorCallTarget;
     }
-
-    /**
-     * Creates a generator function that does not capture any state. Therefore, it can always return
-     * the same generator function instance.
-     */
-    public static final class StatelessGeneratorFunctionDefinitionNode extends GeneratorFunctionDefinitionNode {
-        @CompilationFinal private PGeneratorFunction cached;
-
-        public StatelessGeneratorFunctionDefinitionNode(String name, String enclosingClassName, ExpressionNode doc, Arity arity, RootCallTarget callTarget,
-                        FrameDescriptor frameDescriptor, DefinitionCellSlots definitionCellSlots, ExecutionCellSlots executionCellSlots,
-                        int numOfActiveFlags, int numOfGeneratorBlockNode, int numOfGeneratorForNode) {
-            super(name, enclosingClassName, doc, arity, null, null, callTarget,
-                            frameDescriptor, definitionCellSlots, executionCellSlots,
-                            numOfActiveFlags, numOfGeneratorBlockNode, numOfGeneratorForNode);
-        }
-
-        @Override
-        public PGeneratorFunction execute(VirtualFrame frame) {
-            if (cached == null) {
-                CompilerDirectives.transferToInterpreterAndInvalidate();
-                PCell[] closure = getClosureFromLocals(frame);
-                cached = withDocString(frame,
-                                factory().createGeneratorFunction(functionName, enclosingClassName, arity, getGeneratorCallTarget(closure), PArguments.getGlobals(frame), closure, null, null));
-            }
-            return cached;
-        }
-    }
-
 }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/generator/GeneratorFunctionRootNode.java

@@ -41,7 +41,7 @@
 package com.oracle.graal.python.nodes.generator;
 
 import com.oracle.graal.python.PythonLanguage;
-import com.oracle.graal.python.builtins.objects.cell.PCell;
+import com.oracle.graal.python.builtins.objects.function.PArguments;
 import com.oracle.graal.python.nodes.PClosureFunctionRootNode;
 import com.oracle.graal.python.parser.ExecutionCellSlots;
 import com.oracle.graal.python.runtime.object.PythonObjectFactory;
@@ -56,18 +56,16 @@ public class GeneratorFunctionRootNode extends PClosureFunctionRootNode {
     private final int numOfActiveFlags;
     private final int numOfGeneratorBlockNode;
     private final int numOfGeneratorForNode;
-    private final PCell[] closure;
     private final ExecutionCellSlots cellSlots;
     private final String name;
     @Child private PythonObjectFactory factory = PythonObjectFactory.create();
 
-    public GeneratorFunctionRootNode(PythonLanguage language, RootCallTarget callTarget, String name, FrameDescriptor frameDescriptor, PCell[] closure, ExecutionCellSlots executionCellSlots,
+    public GeneratorFunctionRootNode(PythonLanguage language, RootCallTarget callTarget, String name, FrameDescriptor frameDescriptor, ExecutionCellSlots executionCellSlots,
                     int numOfActiveFlags, int numOfGeneratorBlockNode, int numOfGeneratorForNode) {
         super(language, frameDescriptor, executionCellSlots);
         this.callTarget = callTarget;
         this.name = name;
         this.frameDescriptor = frameDescriptor;
-        this.closure = closure;
         this.cellSlots = executionCellSlots;
         this.numOfActiveFlags = numOfActiveFlags;
         this.numOfGeneratorBlockNode = numOfGeneratorBlockNode;
@@ -76,7 +74,8 @@ public GeneratorFunctionRootNode(PythonLanguage language, RootCallTarget callTar
 
     @Override
     public Object execute(VirtualFrame frame) {
-        return factory.createGenerator(getName(), callTarget, frameDescriptor, frame.getArguments(), closure, cellSlots, numOfActiveFlags, numOfGeneratorBlockNode, numOfGeneratorForNode);
+        return factory.createGenerator(getName(), callTarget, frameDescriptor, frame.getArguments(), PArguments.getClosure(frame), cellSlots, numOfActiveFlags, numOfGeneratorBlockNode,
+                        numOfGeneratorForNode);
     }
 
     public RootNode getFunctionRootNode() {