[GR-76230] Keep function kwdefaults dict live. Fixes oracle/graalpython#561

graalpython/com.oracle.graal.python.test/src/tests/test_functions.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 2026, 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
@@ -192,6 +192,61 @@ def foo(*args, x=1):
     assert_raises(TypeError, foo)
 
 
+def test_function_kwdefaults_dict_is_live():
+    def foo(*, x=1):
+        return x
+
+    kwdefaults = foo.__kwdefaults__
+    assert kwdefaults is foo.__kwdefaults__
+    kwdefaults["x"] = 2
+    assert foo() == 2
+
+    assigned = {"x": 3}
+    foo.__kwdefaults__ = assigned
+    assert foo.__kwdefaults__ is assigned
+    assigned["x"] = 4
+    assert foo() == 4
+
+    assigned[1] = "not a keyword"
+    assert foo.__kwdefaults__ == {"x": 4, 1: "not a keyword"}
+    assert foo() == 4
+
+    non_string_kwdefaults = {1: "not a keyword"}
+    foo.__kwdefaults__ = non_string_kwdefaults
+    assert foo.__kwdefaults__ is non_string_kwdefaults
+    assert_raises(TypeError, foo)
+    non_string_kwdefaults["x"] = 5
+    assert foo() == 5
+
+
+def test_mangled_kwonly_kwdefaults_dict_is_live():
+    class argmap:
+        def make_wrapper(self):
+            def func(*args, __wrapper=None, **kwargs):
+                return __wrapper
+
+            assert "_argmap__wrapper" in func.__kwdefaults__
+            func.__kwdefaults__["_argmap__wrapper"] = func
+            return func
+
+    func = argmap().make_wrapper()
+    assert "_argmap__wrapper" in func.__code__.co_varnames
+    assert "__wrapper" not in func.__code__.co_varnames
+    assert func() is func
+
+
+def test_method_kwdefaults_dict_is_live():
+    class C:
+        def method(self, *, x=1):
+            return x
+
+    obj = C()
+    kwdefaults = obj.method.__kwdefaults__
+    assert kwdefaults is C.method.__kwdefaults__
+    kwdefaults["x"] = 2
+    assert obj.method() == 2
+
+
 def test_code_change():
     def foo():
         return "foo"
@@ -236,7 +291,7 @@ def assign_code(x, y):
     assert_raises(ValueError, assign_code, foo, foobar_code)
     bazbar.__code__ = foobar_code
     assert bazbar() == (2,3)
-    
+
 
 def test_function_dict_writeable():
     def foo(): pass

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/FunctionBuiltins.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2025, Oracle and/or its affiliates.
+ * Copyright (c) 2017, 2026, Oracle and/or its affiliates.
  * Copyright (c) 2014, Regents of the University of California
  *
  * All rights reserved.
@@ -64,6 +64,8 @@
 import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorKey;
 import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorNext;
 import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorValue;
+import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageLen;
+import com.oracle.graal.python.builtins.objects.common.KeywordsStorage;
 import com.oracle.graal.python.builtins.objects.common.SequenceNodes.GetObjectArrayNode;
 import com.oracle.graal.python.builtins.objects.dict.PDict;
 import com.oracle.graal.python.builtins.objects.getsetdescriptor.DescriptorDeleteMarker;
@@ -295,13 +297,13 @@ public abstract static class GetKeywordDefaultsNode extends PythonBinaryBuiltinN
         @Specialization(guards = "isNoValue(arg)")
         static Object get(PFunction self, @SuppressWarnings("unused") PNone arg,
                         @Bind PythonLanguage language) {
-            PKeyword[] kwdefaults = self.getKwDefaults();
-            return (kwdefaults.length > 0) ? PFactory.createDict(language, kwdefaults) : PNone.NONE;
+            return self.getKwDefaultsDict(language);
         }
 
         @Specialization(guards = "!isNoValue(arg)")
-        static Object set(PFunction self, @SuppressWarnings("unused") PNone arg) {
-            self.setKwDefaults(PKeyword.EMPTY_KEYWORDS);
+        static Object set(PFunction self, @SuppressWarnings("unused") PNone arg,
+                        @Bind PythonLanguage language) {
+            self.setKwDefaultsDict(PFactory.createDict(language, PKeyword.EMPTY_KEYWORDS));
             return PNone.NONE;
         }
 
@@ -315,12 +317,16 @@ Object set(PFunction self, PDict arg) {
                 Object key = assertNoJavaString(HashingStorageIteratorKey.executeUncached(storage, it));
                 if (key instanceof PString) {
                     key = ((PString) key).getValueUncached();
-                } else if (!(key instanceof TruffleString)) {
-                    throw PRaiseNode.raiseStatic(this, PythonBuiltinClassType.TypeError, ErrorMessages.KEYWORD_NAMES_MUST_BE_STR_GOT_P, key);
                 }
-                keywords.add(new PKeyword((TruffleString) key, HashingStorageIteratorValue.executeUncached(storage, it)));
+                if (key instanceof TruffleString) {
+                    keywords.add(new PKeyword((TruffleString) key, HashingStorageIteratorValue.executeUncached(storage, it)));
+                }
+            }
+            PKeyword[] kwdefaults = keywords.isEmpty() ? PKeyword.EMPTY_KEYWORDS : keywords.toArray(new PKeyword[keywords.size()]);
+            if (kwdefaults.length == HashingStorageLen.executeUncached(storage)) {
+                arg.setDictStorage(new KeywordsStorage(kwdefaults));
             }
-            self.setKwDefaults(keywords.isEmpty() ? PKeyword.EMPTY_KEYWORDS : keywords.toArray(new PKeyword[keywords.size()]));
+            self.setKwDefaultsDict(arg);
             return PNone.NONE;
         }
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/PFunction.java

@@ -25,16 +25,28 @@
  */
 package com.oracle.graal.python.builtins.objects.function;
 
+import java.util.ArrayList;
+
 import com.oracle.graal.python.PythonLanguage;
 import com.oracle.graal.python.builtins.PythonBuiltinClassType;
 import com.oracle.graal.python.builtins.objects.PNone;
 import com.oracle.graal.python.builtins.objects.cell.PCell;
 import com.oracle.graal.python.builtins.objects.code.PCode;
+import com.oracle.graal.python.builtins.objects.common.HashingStorage;
+import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageGetIterator;
+import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIterator;
+import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorKey;
+import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorNext;
+import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorValue;
+import com.oracle.graal.python.builtins.objects.common.KeywordsStorage;
+import com.oracle.graal.python.builtins.objects.dict.PDict;
 import com.oracle.graal.python.builtins.objects.object.PythonObject;
+import com.oracle.graal.python.builtins.objects.str.PString;
 import com.oracle.graal.python.compiler.CodeUnit;
 import com.oracle.graal.python.lib.PyUnicodeCheckNode;
 import com.oracle.graal.python.nodes.PRootNode;
 import com.oracle.graal.python.runtime.GilNode;
+import com.oracle.graal.python.runtime.object.PFactory;
 import com.oracle.graal.python.util.PythonUtils;
 import com.oracle.truffle.api.Assumption;
 import com.oracle.truffle.api.CompilerAsserts;
@@ -68,6 +80,7 @@ public final class PFunction extends PythonObject {
     private Object[] defaultValues;
     @CompilationFinal(dimensions = 1) private PKeyword[] finalKwDefaultValues;
     private PKeyword[] kwDefaultValues;
+    private PDict kwDefaultDict;
     private Object doc;
 
     public PFunction(PythonLanguage lang, TruffleString name, TruffleString qualname, PCode code, PythonObject globals, PCell[] closure) {
@@ -202,17 +215,53 @@ public void setDefaults(Object[] defaults) {
     public PKeyword[] getKwDefaults() {
         if (CompilerDirectives.inCompiledCode() && CompilerDirectives.isPartialEvaluationConstant(this)) {
             if (codeStableAssumption.isValid()) {
+                assert kwDefaultDict == null;
                 return finalKwDefaultValues;
             }
         }
+        if (kwDefaultDict != null) {
+            HashingStorage storage = kwDefaultDict.getDictStorage();
+            if (CompilerDirectives.injectBranchProbability(CompilerDirectives.LIKELY_PROBABILITY, storage instanceof KeywordsStorage)) {
+                return ((KeywordsStorage) storage).getStore();
+            }
+            return kwDefaultsFromStorage(storage);
+        }
         return kwDefaultValues;
     }
 
+    public Object getKwDefaultsDict(PythonLanguage language) {
+        if (kwDefaultDict == null) {
+            PKeyword[] kwdefaults = getKwDefaults();
+            if (kwdefaults.length == 0) {
+                return PNone.NONE;
+            }
+            setKwDefaultsDict(PFactory.createDict(language, kwdefaults));
+        }
+        return kwDefaultDict;
+    }
+
     @TruffleBoundary
-    public void setKwDefaults(PKeyword[] defaults) {
+    public void setKwDefaultsDict(PDict defaults) {
         this.codeStableAssumption.invalidate("kw defaults changed for function " + getName());
-        this.finalDefaultValues = null; // avoid leak, and make code that wrongly uses it crash
-        this.kwDefaultValues = defaults;
+        this.finalKwDefaultValues = null; // avoid leak, and make code that wrongly uses it crash
+        this.kwDefaultValues = PKeyword.EMPTY_KEYWORDS;
+        this.kwDefaultDict = defaults;
+    }
+
+    @TruffleBoundary
+    private static PKeyword[] kwDefaultsFromStorage(HashingStorage storage) {
+        ArrayList<PKeyword> keywords = new ArrayList<>();
+        HashingStorageIterator it = HashingStorageGetIterator.executeUncached(storage);
+        while (HashingStorageIteratorNext.executeUncached(storage, it)) {
+            Object key = HashingStorageIteratorKey.executeUncached(storage, it);
+            if (key instanceof PString) {
+                key = ((PString) key).getValueUncached();
+            }
+            if (key instanceof TruffleString) {
+                keywords.add(new PKeyword((TruffleString) key, HashingStorageIteratorValue.executeUncached(storage, it)));
+            }
+        }
+        return keywords.isEmpty() ? PKeyword.EMPTY_KEYWORDS : keywords.toArray(new PKeyword[keywords.size()]);
     }
 
     @TruffleBoundary

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/method/MethodBuiltins.java

@@ -66,7 +66,6 @@
 import com.oracle.graal.python.nodes.PRaiseNode;
 import com.oracle.graal.python.nodes.attributes.LookupAttributeInMRONode;
 import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetDefaultsNode;
-import com.oracle.graal.python.nodes.builtins.FunctionNodes.GetKeywordDefaultsNode;
 import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
 import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
 import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
@@ -234,9 +233,13 @@ static Object defaults(PMethod self,
     public abstract static class GetMethodKwdefaultsNode extends PythonUnaryBuiltinNode {
         @Specialization
         static Object kwDefaults(PMethod self,
-                        @Bind Node inliningTarget,
-                        @Cached GetKeywordDefaultsNode getKeywordDefaultsNode) {
-            PKeyword[] kwdefaults = getKeywordDefaultsNode.execute(inliningTarget, self);
+                        @Bind Node inliningTarget) {
+            Object fun = self.getFunction();
+            if (fun instanceof PFunction function) {
+                return function.getKwDefaultsDict(PythonLanguage.get(inliningTarget));
+            }
+            assert fun instanceof PBuiltinFunction;
+            PKeyword[] kwdefaults = ((PBuiltinFunction) fun).getKwDefaults();
             return (kwdefaults.length > 0) ? PFactory.createDict(PythonLanguage.get(inliningTarget), kwdefaults) : PNone.NONE;
         }
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/ErrorMessages.java

@@ -443,7 +443,6 @@ public abstract class ErrorMessages {
     public static final TruffleString ITERATION_VALUE_MUST_BE_GREATER_THAN_ZERO = tsLiteral("iteration value must be greater than 0.");
     public static final TruffleString ITERATION_VALUE_IS_TOO_GREAT = tsLiteral("iteration value is too great.");
     public static final TruffleString KEY_LENGTH_MUST_BE_GREATER_THAN_ZERO = tsLiteral("key length must be greater than 0.");
-    public static final TruffleString KEYWORD_NAMES_MUST_BE_STR_GOT_P = tsLiteral("keyword names must be str, get %p");
     public static final TruffleString KEYWORDS_S_MUST_BE_STRINGS = tsLiteral("keywords must be strings");
     public static final TruffleString KLASS_ARG_IS_NOT_HOST_OBJ = tsLiteral("instanceof second argument '%p' is not a Java class");
     public static final TruffleString LAZY_INITIALIZATION_FAILED = tsLiteral("lazy initialization of type %N failed");

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/builtins/FunctionNodes.java

@@ -44,7 +44,6 @@
 import com.oracle.graal.python.builtins.objects.code.PCode;
 import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
 import com.oracle.graal.python.builtins.objects.function.PFunction;
-import com.oracle.graal.python.builtins.objects.function.PKeyword;
 import com.oracle.graal.python.builtins.objects.function.Signature;
 import com.oracle.graal.python.builtins.objects.method.PBuiltinMethod;
 import com.oracle.graal.python.builtins.objects.method.PMethod;
@@ -126,66 +125,6 @@ static Object[] doBuiltinMethod(PBuiltinMethod builtinMethod) {
 
     }
 
-    @ImportStatic(PGuards.class)
-    @GenerateUncached
-    @GenerateInline
-    @GenerateCached(false)
-    public abstract static class GetKeywordDefaultsNode extends PNodeWithContext {
-
-        public abstract PKeyword[] execute(Node inliningTarget, Object function);
-
-        public abstract PKeyword[] execute(Node inliningTarget, PMethodBase method);
-
-        /**
-         * Fast-path if method is a partial evaluation constant.
-         */
-        public static PKeyword[] getMethodKeywords(PMethodBase method) {
-            CompilerAsserts.partialEvaluationConstant(method);
-            return getFunctionKeywords(method.getFunction());
-        }
-
-        /**
-         * Fast-path if the function is a partial evaluation constant.
-         */
-        public static PKeyword[] getFunctionKeywords(Object fun) {
-            CompilerAsserts.partialEvaluationConstant(fun);
-            if (fun instanceof PFunction f) {
-                return f.getKwDefaults();
-            } else if (fun instanceof PBuiltinFunction f) {
-                return f.getKwDefaults();
-            }
-            throw CompilerDirectives.shouldNotReachHere();
-        }
-
-        @Specialization
-        static PKeyword[] doFunction(PFunction function) {
-            return function.getKwDefaults();
-        }
-
-        @Specialization
-        static PKeyword[] doBuiltinFunction(PBuiltinFunction builtinFunction) {
-            return builtinFunction.getKwDefaults();
-        }
-
-        @Specialization(guards = "isPFunction(function)")
-        static PKeyword[] doMethod(@SuppressWarnings("unused") PMethod method,
-                        @Bind("method.getFunction()") Object function) {
-            return ((PFunction) function).getKwDefaults();
-        }
-
-        @Specialization(guards = "isPBuiltinFunction(method.getFunction())")
-        static PKeyword[] doMethodBuiltin(@SuppressWarnings("unused") PMethod method,
-                        @Bind("method.getFunction()") Object function) {
-            return ((PBuiltinFunction) function).getKwDefaults();
-        }
-
-        @Specialization
-        static PKeyword[] doBuiltinMethod(PBuiltinMethod builtinMethod) {
-            return builtinMethod.getBuiltinFunction().getKwDefaults();
-        }
-
-    }
-
     @GenerateUncached
     @GenerateInline
     @GenerateCached(false)