[GR-55196] Implement tp_hash and tp_richcompare.

PullRequest: graalpython/3628

Author: steve-s

graalpython/com.oracle.graal.python.annotations/src/com/oracle/graal/python/annotations/HashNotImplemented.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2024, 2025, 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.
+ */
+package com.oracle.graal.python.annotations;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Indicates that the annotated class inheriting {@code PythonBuiltins} should have slot
+ * {@code tp_hash} filled with the {@code PyObject_HashNotImplemented} placeholder. There is some
+ * special inheritance semantics for this specific slot value.
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface HashNotImplemented {
+}

graalpython/com.oracle.graal.python.annotations/src/com/oracle/graal/python/annotations/Slot.java

@@ -185,10 +185,16 @@ enum SlotKind {
         mp_subscript("__getitem__"),
         /** o[key] = value */
         mp_ass_subscript("__setitem__"),
+        /** comparison operations: >,=>, ==, !=, <, <= */
+        tp_richcompare("__lt__, __le__, __eq__, __ne__, __gt__, __ge__"),
         /** type descriptor get */
         tp_descr_get("__get__"),
         /** type descriptor set/delete */
         tp_descr_set("__set__, __delete__"),
+        /**
+         * hash code. See also if {@link HashNotImplemented} is not more appropriate.
+         */
+        tp_hash("__hash__"),
         /** get object attribute */
         tp_getattro("__getattribute__, __getattr__"),
         /** set/delete object attribute */

graalpython/com.oracle.graal.python.cext/src/typeobject.c

@@ -1,4 +1,4 @@
-/* Copyright (c) 2018, 2024, Oracle and/or its affiliates.
+/* Copyright (c) 2018, 2025, Oracle and/or its affiliates.
  * Copyright (C) 1996-2022 Python Software Foundation
  *
  * Licensed under the PYTHON SOFTWARE FOUNDATION LICENSE VERSION 2
@@ -9362,13 +9362,8 @@ static int type_ready_graalpy_slot_conv(PyTypeObject* cls) {
     ADD_SLOT_CONV("__delattr__", cls->tp_setattro, -3, JWRAPPER_DELATTRO);
     ADD_SLOT_CONV("__clear__", cls->tp_clear, -1, JWRAPPER_INQUIRY);
 
-    /* IMPORTANT NOTE: If the class already provides 'tp_richcompare' but this is the default
-       'object.__truffle_richcompare__' function, then we need to break a recursive cycle since
-       the default function dispatches to the individual comparison functions which would in
-       this case again invoke 'object.__truffle_richcompare__'. */
     richcmpfunc richcompare = cls->tp_richcompare;
-    if (richcompare && richcompare != PyBaseObject_Type.tp_richcompare) {
-        ADD_SLOT_CONV("__compare__", richcompare, -3, JWRAPPER_RICHCMP);
+    if (richcompare) {
         ADD_SLOT_CONV("__lt__", richcompare, -2, JWRAPPER_LT);
         ADD_SLOT_CONV("__le__", richcompare, -2, JWRAPPER_LE);
         ADD_SLOT_CONV("__eq__", richcompare, -2, JWRAPPER_EQ);

graalpython/com.oracle.graal.python.processor/src/com/oracle/graal/python/processor/ConverterFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2025, 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
@@ -210,7 +210,8 @@ public static ConverterFactory[] getForClass(TypeElement conversionClass) throws
     private static ConverterFactory[] forBuiltin(Elements elementUtils, String className) throws ProcessingError {
         TypeElement type = elementUtils.getTypeElement(CLINIC_PACKAGE + "." + className);
         if (type == null) {
-            throw new ProcessingError(null, "Unable to find built-in argument clinic conversion node " + CLINIC_PACKAGE + "." + className);
+            throw new ProcessingError(null, "Unable to find built-in argument clinic conversion node " + CLINIC_PACKAGE + "." + className +
+                            ". This may be also a sign that Truffle DSL annotation processor failed to process this class.");
         }
         return getForClass(type);
     }

graalpython/com.oracle.graal.python.processor/src/com/oracle/graal/python/processor/SlotsMapping.java

@@ -73,15 +73,18 @@ static String getSlotBaseClass(Slot s) {
             case mp_subscript -> "TpSlotBinaryFunc.TpSlotMpSubscript";
             case mp_ass_subscript -> "TpSlotMpAssSubscript.TpSlotMpAssSubscriptBuiltin";
             case tp_getattro -> "TpSlotGetAttr.TpSlotGetAttrBuiltin";
+            case tp_richcompare -> "TpSlotRichCompare.TpSlotRichCmpBuiltin" + getSuffix(s.isComplex());
             case tp_descr_get -> "TpSlotDescrGet.TpSlotDescrGetBuiltin" + getSuffix(s.isComplex());
             case tp_descr_set -> "TpSlotDescrSet.TpSlotDescrSetBuiltin";
             case tp_setattro -> "TpSlotSetAttr.TpSlotSetAttrBuiltin";
             case tp_iternext -> "TpSlotIterNext.TpSlotIterNextBuiltin";
+            case tp_hash -> "TpSlotHashFun.TpSlotHashBuiltin";
         };
     }
 
     static String getSlotNodeBaseClass(Slot s) {
         return switch (s.value()) {
+            case tp_richcompare -> "com.oracle.graal.python.builtins.objects.type.slots.TpSlotRichCompare.RichCmpBuiltinNode";
             case tp_descr_get -> "com.oracle.graal.python.builtins.objects.type.slots.TpSlotDescrGet.DescrGetBuiltinNode";
             case nb_bool -> "com.oracle.graal.python.builtins.objects.type.slots.TpSlotInquiry.NbBoolBuiltinNode";
             case nb_index, nb_int, nb_float, nb_absolute, nb_positive, nb_negative, nb_invert,
@@ -107,12 +110,14 @@ static String getSlotNodeBaseClass(Slot s) {
             case tp_getattro -> "com.oracle.graal.python.builtins.objects.type.slots.TpSlotGetAttr.GetAttrBuiltinNode";
             case tp_descr_set -> "com.oracle.graal.python.builtins.objects.type.slots.TpSlotDescrSet.DescrSetBuiltinNode";
             case tp_setattro -> "com.oracle.graal.python.builtins.objects.type.slots.TpSlotSetAttr.SetAttrBuiltinNode";
+            case tp_hash -> "com.oracle.graal.python.builtins.objects.type.slots.TpSlotHashFun.HashBuiltinNode";
         };
     }
 
     static String getUncachedExecuteSignature(SlotKind s) {
         return switch (s) {
             case nb_bool -> "boolean executeUncached(Object self)";
+            case tp_richcompare -> "Object executeUncached(Object self, Object obj, com.oracle.graal.python.lib.RichCmpOp op)";
             case tp_descr_get -> "Object executeUncached(Object self, Object obj, Object type)";
             case sq_length, mp_length -> "int executeUncached(Object self)";
             default -> throw new AssertionError("Should not reach here: should be always complex");
@@ -128,14 +133,15 @@ static boolean supportsComplex(SlotKind s) {
 
     static boolean supportsSimple(SlotKind s) {
         return switch (s) {
-            case nb_bool, sq_length, mp_length, tp_descr_get -> true;
+            case nb_bool, sq_length, mp_length, tp_descr_get, tp_richcompare -> true;
             default -> false;
         };
     }
 
     static String getUncachedExecuteCall(SlotKind s) {
         return switch (s) {
             case nb_bool -> "executeBool(null, self)";
+            case tp_richcompare -> "execute(null, self, obj, op)";
             case sq_length, mp_length -> "executeInt(null, self)";
             case tp_descr_get -> "execute(null, self, obj, type)";
             default -> throw new AssertionError("Should not reach here: should be always complex");

graalpython/com.oracle.graal.python.processor/src/com/oracle/graal/python/processor/SlotsProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024, 2025, 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
@@ -63,6 +63,7 @@
 import javax.tools.Diagnostic.Kind;
 import javax.tools.JavaFileObject;
 
+import com.oracle.graal.python.annotations.HashNotImplemented;
 import com.oracle.graal.python.annotations.Slot;
 import com.oracle.graal.python.annotations.Slot.SlotKind;
 import com.oracle.graal.python.annotations.Slot.Slots;
@@ -72,11 +73,27 @@ public class SlotsProcessor extends AbstractProcessor {
     private static final boolean LOGGING = false;
 
     public record TpSlotData(Slot slot, TypeElement enclosingType, TypeElement slotNodeType) {
+        public boolean isHashNotImplemented() {
+            return slotNodeType == null;
+        }
+
+        public static TpSlotData createHashNotImplemented(TypeElement enclosingType) {
+            return new TpSlotData(null, enclosingType, null);
+        }
+
+        public String builderCall() {
+            if (isHashNotImplemented()) {
+                return ".set(TpSlots.TpSlotMeta.TP_HASH, com.oracle.graal.python.builtins.objects.type.slots.TpSlotHashFun.HASH_NOT_IMPLEMENTED)";
+            }
+            return String.format(".set(TpSlots.TpSlotMeta.%s, %s.INSTANCE)", //
+                            slot.value().name().toUpperCase(Locale.ROOT), //
+                            getSlotImplName(slot.value()));
+        }
     }
 
     @Override
     public Set<String> getSupportedAnnotationTypes() {
-        return Set.of(Slot.class.getName());
+        return Set.of(Slot.class.getName(), HashNotImplemented.class.getName());
     }
 
     @Override
@@ -109,7 +126,14 @@ private void doProcess(RoundEnvironment roundEnv) throws IOException, Processing
     private void validate(HashMap<TypeElement, Set<TpSlotData>> enclosingTypes) throws ProcessingError {
         var typeCache = new TypeCache(processingEnv);
         for (Entry<TypeElement, Set<TpSlotData>> enclosingType : enclosingTypes.entrySet()) {
+            boolean seenHashNotImplemented = false;
+            boolean seenHash = false;
             for (TpSlotData slot : enclosingType.getValue()) {
+                if (slot.isHashNotImplemented()) {
+                    seenHashNotImplemented = true;
+                    continue;
+                }
+                seenHash |= slot.slot.value() == SlotKind.tp_hash;
                 if (slot.slot.isComplex()) {
                     if (!SlotsMapping.supportsComplex(slot.slot().value())) {
                         throw error(slot.slotNodeType, "Slot does not support complex builtins. The support can be added.");
@@ -125,14 +149,22 @@ private void validate(HashMap<TypeElement, Set<TpSlotData>> enclosingTypes) thro
                     throw error(slot.slotNodeType, "Slot does not inherit from expected base class '%s'", baseName);
                 }
             }
+            if (seenHash && seenHashNotImplemented) {
+                throw error(enclosingType.getKey(), "Annotation %s cannot be use when there is also %s(tp_hash). Remove one or the other.",
+                                HashNotImplemented.class.getSimpleName(),
+                                Slot.class.getSimpleName());
+            }
         }
     }
 
     @SuppressWarnings("try")
     private void writeCode(HashMap<TypeElement, Set<TpSlotData>> enclosingTypes) throws IOException {
-        for (Entry<TypeElement, Set<TpSlotData>> enclosingType : enclosingTypes.entrySet()) {
-            String pkgName = getPackage(enclosingType.getKey());
-            String className = enclosingType.getKey().getSimpleName() + "SlotsGen";
+        for (Entry<TypeElement, Set<TpSlotData>> enclosingTypeAndSlots : enclosingTypes.entrySet()) {
+            Set<TpSlotData> slots = enclosingTypeAndSlots.getValue();
+            TypeElement enclosingType = enclosingTypeAndSlots.getKey();
+
+            String pkgName = getPackage(enclosingType);
+            String className = enclosingType.getSimpleName() + "SlotsGen";
             String sourceFile = pkgName + "." + className;
             log("Generating file '%s'", sourceFile);
 
@@ -146,10 +178,10 @@ private void writeCode(HashMap<TypeElement, Set<TpSlotData>> enclosingTypes) thr
                 w.writeLn();
                 w.writeLn("public class %s {", className);
                 try (Block i = w.newIndent()) {
-                    for (TpSlotData slot : enclosingType.getValue()) {
+                    for (TpSlotData slot : slots) {
                         writeSlot(w, slot);
                     }
-                    writeSlotsStaticField(w, enclosingType.getValue());
+                    writeSlotsStaticField(w, enclosingType, slots);
                 }
                 w.writeLn("}");
             }
@@ -168,6 +200,9 @@ private void writeImports(CodeWriter w) throws IOException {
 
     @SuppressWarnings("try")
     private void writeSlot(CodeWriter w, TpSlotData slot) throws IOException {
+        if (slot.isHashNotImplemented()) {
+            return;
+        }
         log("Writing slot node %s", slot.slotNodeType);
         String slotImplName = getSlotImplName(slot.slot.value());
         String genericArg = "";
@@ -202,13 +237,11 @@ private void writeSlot(CodeWriter w, TpSlotData slot) throws IOException {
     }
 
     @SuppressWarnings("try")
-    private static void writeSlotsStaticField(CodeWriter w, Set<TpSlotData> slots) throws IOException {
+    private static void writeSlotsStaticField(CodeWriter w, TypeElement enclosingType, Set<TpSlotData> slots) throws IOException {
         w.writeLn("static final TpSlots SLOTS = TpSlots.newBuilder()");
         try (Block i3 = w.newIndent()) {
             String defs = slots.stream().//
-                            map(s -> String.format(".set(TpSlots.TpSlotMeta.%s, %s.INSTANCE)", //
-                                            s.slot.value().name().toUpperCase(Locale.ROOT), //
-                                            getSlotImplName(s.slot.value()))).//
+                            map(TpSlotData::builderCall).//
                             collect(Collectors.joining("\n"));
             w.writeLn("%s.", defs);
             w.writeLn("build();");
@@ -237,6 +270,12 @@ private HashMap<TypeElement, Set<TpSlotData>> collectEnclosingTypes(RoundEnviron
                 tpSlotDataSet.add(new TpSlotData(slotAnnotation, enclosingType, type));
             }
         }
+        elements = new HashSet<>(roundEnv.getElementsAnnotatedWithAny(Set.of(HashNotImplemented.class)));
+        for (Element element : elements) {
+            TypeElement typeElement = (TypeElement) element;
+            enclosingTypes.computeIfAbsent(typeElement, key -> new HashSet<>()).//
+                            add(TpSlotData.createHashNotImplemented(typeElement));
+        }
         return enclosingTypes;
     }
 

graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/objects/ObjectHashMapTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2025, 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
@@ -52,6 +52,7 @@
 import java.util.Random;
 import java.util.stream.Collectors;
 
+import com.oracle.graal.python.lib.RichCmpOp;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -69,7 +70,6 @@
 import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PopNode;
 import com.oracle.graal.python.lib.PyObjectHashNode;
 import com.oracle.graal.python.lib.PyObjectRichCompareBool;
-import com.oracle.graal.python.lib.PyObjectRichCompareBool.Comparison;
 import com.oracle.truffle.api.frame.Frame;
 import com.oracle.truffle.api.interop.TruffleObject;
 import com.oracle.truffle.api.nodes.Node;
@@ -86,9 +86,9 @@ public static final class DictKey implements TruffleObject {
         }
     }
 
-    private static final class EqNodeStub extends PyObjectRichCompareBool.EqNode {
+    private static final class EqNodeStub extends PyObjectRichCompareBool {
         @Override
-        protected boolean execute(Frame frame, Node inliningTarget, Object a, Object b, Comparison cmp) {
+        public boolean execute(Frame frame, Node inliningTarget, Object a, Object b, RichCmpOp cmp) {
             // Sanity check: we do not use any other keys in the tests
             assert a instanceof Long || a instanceof DictKey;
             assert b instanceof Long || b instanceof DictKey;