Merge pull request #4519 in G/graalpython from ss/slots-tp_descrget-compat-GR-53082 to master

* commit '0dd805aee99f11b59285b0fa27c8e8af6d1f75ba':
  Align method attribute lookup with CPython GR-53090
  Align method descriptor behavior with CPython
  Align classmethod descriptor behavior with CPython

Author: steve-s

graalpython/com.oracle.graal.python.test/src/tests/test_call-classmethod.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2018, 2021, Oracle and/or its affiliates.
+# Copyright (c) 2018, 2026, Oracle and/or its affiliates.
 # Copyright (c) 2013, Regents of the University of California
 #
 # All rights reserved.
@@ -47,3 +47,137 @@ def test_strength():
     assert not Strength.weaker(s1, s2)
     assert s1.stronger(s1, s2)
     assert not s1.weaker(s1, s2)
+
+
+def test_classmethod_wraps_descriptor():
+    class Descriptor:
+        def __get__(self, obj, typ=None):
+            return obj, typ
+
+    class C:
+        method = classmethod(Descriptor())
+
+    assert C.method == (C, C)
+    assert C().method == (C, C)
+
+    method = C.__dict__["method"]
+    assert method.__get__(None, C) == (C, C)
+    assert method.__get__(C()) == (C, C)
+
+
+def test_classmethod_wraps_property():
+    class C:
+        @classmethod
+        @property
+        def name(cls):
+            return cls.__name__
+
+    class D(C):
+        pass
+
+    assert C.name == "C"
+    assert C().name == "C"
+    assert D.name == "D"
+    assert D().name == "D"
+
+
+def test_classmethod_wraps_staticmethod():
+    class C:
+        method = classmethod(staticmethod(lambda value: ("static", value)))
+
+    assert C.method("arg") == ("static", "arg")
+    assert C().method("arg") == ("static", "arg")
+
+
+def test_classmethod_wraps_classmethod():
+    class C:
+        def method(cls, value):
+            return cls, value
+
+        method = classmethod(classmethod(method))
+
+    class D(C):
+        pass
+
+    assert C.method("arg") == (C, "arg")
+    assert C().method("arg") == (C, "arg")
+    assert D.method("arg") == (D, "arg")
+    assert D().method("arg") == (D, "arg")
+
+
+def test_classmethod_wraps_bound_method():
+    class C:
+        def method(self, cls):
+            return self, cls
+
+    receiver = C()
+    assert not hasattr(type(receiver.method), "__get__")
+
+    class D:
+        method = classmethod(receiver.method)
+
+    assert D.method() == (receiver, D)
+    assert D().method() == (receiver, D)
+
+
+def test_classmethod_descriptor_get_errors():
+    descriptor = dict.__dict__["fromkeys"]
+
+    assert descriptor.__get__(None, dict)([1, 2]) == {1: None, 2: None}
+    assert descriptor.__get__({})([1, 2]) == {1: None, 2: None}
+
+    for args in ((None, None), (42,), (None, 42), (None, int), ({}, int)):
+        try:
+            descriptor.__get__(*args)
+        except TypeError:
+            pass
+        else:
+            raise AssertionError("classmethod_descriptor.__get__ accepted invalid arguments")
+
+
+def test_classmethod_descriptor_get_uses_object_type_when_type_omitted():
+    class MyDict(dict):
+        pass
+
+    descriptor = dict.__dict__["fromkeys"]
+    bound = descriptor.__get__(MyDict())
+    assert bound.__self__ is MyDict
+    assert type(bound([1, 2])) is MyDict
+
+
+def test_classmethod_descriptor_get_does_not_keep_type_alive():
+    import time
+    from test import support
+
+    descriptor = object.__dict__["__init_subclass__"]
+
+    class Parent:
+        pass
+
+    class Child(Parent):
+        pass
+
+    bound = descriptor.__get__(None, Child)
+    del bound
+    assert Parent.__subclasses__() == [Child]
+
+    del Child
+    for _ in range(100):
+        support.gc_collect()
+        if not Parent.__subclasses__():
+            break
+        if getattr(support, "is_graalpy", False):
+            time.sleep(0.1)
+    assert Parent.__subclasses__() == []
+
+
+def test_cext_classmethod_descriptor():
+    from _ctypes import _SimpleCData
+
+    class c_void_p(_SimpleCData):
+        _type_ = "P"
+
+    descriptor = c_void_p.__dict__["from_param"]
+    assert type(descriptor).__name__ == "classmethod_descriptor"
+    assert callable(descriptor.__get__(None, c_void_p))
+    c_void_p.from_param(0)

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

@@ -107,3 +107,35 @@ def g(self):
     assert types.MethodType(f, A).__qualname__ == "test_method_qualname_uses_wrapped_callable.<locals>.f"
     assert A.m.__qualname__ == "test_method_qualname_uses_wrapped_callable.<locals>.f"
     assert A.__dict__["m"].__qualname__ == "test_method_qualname_uses_wrapped_callable.<locals>.f"
+
+
+def test_method_getattribute_does_not_swallow_method_descriptor_attribute_error():
+    import types
+
+    class Callable:
+        def __init__(self):
+            self.name_calls = 0
+
+        def __call__(self, *args):
+            pass
+
+        def __getattribute__(self, name):
+            if name == "__name__":
+                calls = object.__getattribute__(self, "name_calls")
+                object.__setattr__(self, "name_calls", calls + 1)
+                if calls == 0:
+                    raise AttributeError("first lookup failure")
+                return "fallback-name"
+            return object.__getattribute__(self, name)
+
+    func = Callable()
+    method = types.MethodType(func, object())
+    try:
+        method.__name__
+    except AttributeError as e:
+        assert str(e) == "first lookup failure"
+    else:
+        assert False, "AttributeError was not raised"
+    assert func.name_calls == 1
+    assert method.__class__ is types.MethodType
+    assert method.__name__ == "fallback-name"

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextDescrBuiltins.java

@@ -104,7 +104,7 @@ public static long GraalPyPrivate_Descr_NewClassMethod(long methodDefPtr, long n
         Object type = NativeToPythonClassInternalNode.executeUncached(typeRaw);
         PBuiltinFunction func = MethodDescriptorWrapper.createWrapperFunction(language, name, methPtr, type, flags);
         assert func != null;
-        PDecoratedMethod classMethod = PFactory.createClassmethodFromCallableObj(language, func);
+        PDecoratedMethod classMethod = PFactory.createBuiltinClassmethodFromCallableObj(language, func);
         WriteAttributeToPythonObjectNode.executeUncached(classMethod, T___NAME__, name);
         WriteAttributeToPythonObjectNode.executeUncached(classMethod, T___DOC__, doc);
         HiddenAttr.WriteLongNode.executeUncached(classMethod, METHOD_DEF_PTR, methodDefPtr);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTypeBuiltins.java

@@ -298,7 +298,7 @@ private static PythonBuiltinObject typeAddMethod(PythonLanguage language, long m
                 throw PRaiseNode.raiseStatic(EncapsulatingNodeReference.getCurrent().get(), PythonBuiltinClassType.ValueError, ErrorMessages.METHOD_CANNOT_BE_BOTH_CLASS_AND_STATIC);
             }
             assert func != null;
-            return PFactory.createClassmethodFromCallableObj(language, func);
+            return PFactory.createBuiltinClassmethodFromCallableObj(language, func);
         } else if (CExtContext.isMethStatic(flags)) {
             return PFactory.createStaticmethodFromCallableObj(language, func);
         }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 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
@@ -61,21 +61,35 @@
 import com.oracle.graal.python.builtins.PythonBuiltinClassType;
 import com.oracle.graal.python.builtins.PythonBuiltins;
 import com.oracle.graal.python.builtins.objects.PNone;
+import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
 import com.oracle.graal.python.builtins.objects.str.StringUtils.SimpleTruffleStringFormatNode;
 import com.oracle.graal.python.builtins.objects.type.TpSlots;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes;
+import com.oracle.graal.python.builtins.objects.type.slots.TpSlotDescrGet.DescrGetBuiltinNode;
 import com.oracle.graal.python.lib.PyObjectGetAttr;
 import com.oracle.graal.python.lib.PyObjectLookupAttr;
 import com.oracle.graal.python.lib.PyObjectStrAsTruffleStringNode;
+import com.oracle.graal.python.nodes.ErrorMessages;
+import com.oracle.graal.python.nodes.PGuards;
+import com.oracle.graal.python.nodes.PRaiseNode;
+import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
 import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
 import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
+import com.oracle.graal.python.nodes.object.GetClassNode;
+import com.oracle.graal.python.runtime.exception.PException;
+import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
+import com.oracle.truffle.api.HostCompilerDirectives.InliningCutoff;
 import com.oracle.truffle.api.dsl.Bind;
 import com.oracle.truffle.api.dsl.Cached;
+import com.oracle.truffle.api.dsl.Cached.Exclusive;
 import com.oracle.truffle.api.dsl.GenerateNodeFactory;
+import com.oracle.truffle.api.dsl.GenerateUncached;
 import com.oracle.truffle.api.dsl.NodeFactory;
+import com.oracle.truffle.api.dsl.ReportPolymorphism;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.api.profiles.InlinedBranchProfile;
 import com.oracle.truffle.api.strings.TruffleString;
 
 @CoreFunctions(extendClasses = PythonBuiltinClassType.PBuiltinClassMethod)
@@ -88,6 +102,97 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
         return BuiltinClassmethodBuiltinsFactory.getFactories();
     }
 
+    @Slot(SlotKind.tp_descr_get)
+    @ReportPolymorphism
+    @GenerateUncached
+    @GenerateNodeFactory
+    abstract static class GetNode extends DescrGetBuiltinNode {
+        // If self.getCallable() is null, let the next @Specialization handle that
+        @Specialization(guards = {"isSingleContext()", "isNoValue(type) == typeIsNoValue", "cachedSelf == self", "cachedCallable != null"}, limit = "3")
+        static Object getCached(@SuppressWarnings("unused") PDecoratedMethod self, Object obj, Object type,
+                        @Bind Node inliningTarget,
+                        @SuppressWarnings("unused") @Cached(value = "self", weak = true) PDecoratedMethod cachedSelf,
+                        @Cached(value = "self.getCallable()", weak = true) Object cachedCallable,
+                        @Cached("isNoValue(type)") boolean typeIsNoValue,
+                        @Exclusive @Cached GetClassNode getClass,
+                        @Exclusive @Cached IsSubtypeNode isSubtypeNode,
+                        @Exclusive @Cached InlinedBranchProfile errorProfile,
+                        @Exclusive @Cached ClassmethodCommonBuiltins.MakeMethodNode makeMethod) {
+            Object actualType = getType(inliningTarget, errorProfile, getClass, cachedCallable, typeIsNoValue, obj, type);
+            return doGet(inliningTarget, isSubtypeNode, errorProfile, makeMethod, actualType, cachedCallable);
+        }
+
+        @InliningCutoff
+        @Specialization(replaces = "getCached")
+        static Object get(PDecoratedMethod self, Object obj, Object type,
+                        @Bind Node inliningTarget,
+                        @Exclusive @Cached GetClassNode getClass,
+                        @Exclusive @Cached IsSubtypeNode isSubtypeNode,
+                        @Exclusive @Cached InlinedBranchProfile errorProfile,
+                        @Exclusive @Cached ClassmethodCommonBuiltins.MakeMethodNode makeMethod,
+                        @Exclusive @Cached PRaiseNode raiseNode) {
+            Object callable = ClassmethodCommonBuiltins.getCallable(inliningTarget, self, raiseNode);
+            Object actualType = getType(inliningTarget, errorProfile, getClass, callable, PGuards.isNoValue(type), obj, type);
+            return doGet(inliningTarget, isSubtypeNode, errorProfile, makeMethod, actualType, callable);
+        }
+
+        private static Object getType(Node inliningTarget, InlinedBranchProfile errorProfile, GetClassNode getClassNode,
+                        Object callable, boolean typeIsNoValue, Object obj, Object type) {
+            if (typeIsNoValue) {
+                if (PGuards.isNoValue(obj)) {
+                    errorProfile.enter(inliningTarget);
+                    throw raiseNeedsEitherObjOrType(inliningTarget, callable);
+                }
+                return getClassNode.execute(inliningTarget, obj);
+            }
+            return type;
+        }
+
+        @TruffleBoundary
+        private static PException raiseNeedsEitherObjOrType(Node inliningTarget, Object callable) {
+            if (callable instanceof PBuiltinFunction pbf) {
+                throw PRaiseNode.raiseStatic(inliningTarget, PythonBuiltinClassType.TypeError,
+                                ErrorMessages.DESCRIPTOR_S_FOR_TYPE_S_NEEDS_EITHER_OBJ_OR_TYPE, pbf.getName(),
+                                TypeNodes.GetNameNode.executeUncached(pbf.getEnclosingType()));
+            } else {
+                throw PRaiseNode.raiseStatic(inliningTarget, PythonBuiltinClassType.TypeError);
+            }
+        }
+
+        private static Object doGet(Node inliningTarget, IsSubtypeNode isSubtypeNode, InlinedBranchProfile errorProfile,
+                        ClassmethodCommonBuiltins.MakeMethodNode makeMethod, Object type, Object callable) {
+            if (!PGuards.isPythonClass(type)) {
+                errorProfile.enter(inliningTarget);
+                throw raiseNeedsType(inliningTarget, callable, type);
+            }
+            // Not clear if we can get any other callable than PBuiltinFunction...
+            if (callable instanceof PBuiltinFunction builtinFunction) {
+                Object descriptorType = builtinFunction.getEnclosingType();
+                if (!isSubtypeNode.execute(type, descriptorType)) {
+                    errorProfile.enter(inliningTarget);
+                    throw raiseRequiresSubtype(inliningTarget, builtinFunction, descriptorType, type);
+                }
+            }
+            return makeMethod.execute(inliningTarget, type, callable);
+        }
+
+        @TruffleBoundary
+        private static RuntimeException raiseNeedsType(Node inliningTarget, Object callable, Object type) {
+            if (callable instanceof PBuiltinFunction pbf) {
+                throw PRaiseNode.raiseStatic(inliningTarget, PythonBuiltinClassType.TypeError, ErrorMessages.DESCRIPTOR_S_FOR_TYPE_S_NEEDS_TYPE_NOT_P_AS_ARG_2,
+                                pbf.getName(), TypeNodes.GetNameNode.executeUncached(pbf.getEnclosingType()), type);
+            } else {
+                throw PRaiseNode.raiseStatic(inliningTarget, PythonBuiltinClassType.TypeError);
+            }
+        }
+
+        @TruffleBoundary
+        private static RuntimeException raiseRequiresSubtype(Node inliningTarget, PBuiltinFunction builtinFunction, Object descriptorType, Object type) {
+            throw PRaiseNode.raiseStatic(inliningTarget, PythonBuiltinClassType.TypeError, ErrorMessages.DESCRIPTOR_S_REQUIRES_SUBTYPE_OF_S_BUT_RECEIVED_S,
+                            builtinFunction.getName(), TypeNodes.GetNameNode.executeUncached(descriptorType), TypeNodes.GetNameNode.executeUncached(type));
+        }
+    }
+
     @Builtin(name = J___NAME__, maxNumOfPositionalArgs = 1, isGetter = true)
     @GenerateNodeFactory
     abstract static class NameNode extends PythonUnaryBuiltinNode {