Fix: __ipow__ is always called as binary

Author: fangerer

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/hpy/GraalHPyContextFunctions.java

@@ -392,7 +392,6 @@ static final class CallArithmeticRootNode extends PRootNode {
         private static final Signature SIGNATURE_UNARY = new Signature(1, false, -1, false, new String[]{"$self"}, null);
         private static final Signature SIGNATURE_BINARY = new Signature(2, false, -1, false, new String[]{"$self", "other"}, null);
         private static final Signature SIGNATURE_TERNARY = new Signature(3, false, -1, false, new String[]{"x", "y", "z"}, null);
-        private static final Signature SIGNATURE_INPLACE = new Signature(3, false, -1, false, new String[]{"x", "y"}, null);
 
         @Child private LookupAndCallUnaryNode callUnaryNode;
         @Child private LookupAndCallBinaryNode callBinaryNode;
@@ -422,9 +421,7 @@ public Signature getSignature() {
                 return SIGNATURE_UNARY;
             } else if (binaryOperator != null) {
                 return SIGNATURE_BINARY;
-            } else if (inplaceOperator != null) {
-                return SIGNATURE_INPLACE;
-            } else if (ternaryOperator != null) {
+            } else if (inplaceOperator != null || ternaryOperator != null) {
                 return SIGNATURE_TERNARY;
             } else {
                 throw CompilerDirectives.shouldNotReachHere();
@@ -455,7 +452,13 @@ public Object execute(VirtualFrame frame) {
                 } else if (binaryOperator != null) {
                     return callBinaryNode.executeObject(frame, PArguments.getArgument(frame, 0), PArguments.getArgument(frame, 1));
                 } else if (inplaceOperator != null) {
-                    return callInplaceNode.execute(frame, PArguments.getArgument(frame, 0), PArguments.getArgument(frame, 1));
+                    // most of the in-place operators are binary but there can also be ternary
+                    if (PArguments.getUserArgumentLength(frame) == 2) {
+                        return callInplaceNode.execute(frame, PArguments.getArgument(frame, 0), PArguments.getArgument(frame, 1));
+                    } else if (PArguments.getUserArgumentLength(frame) == 3) {
+                        return callInplaceNode.executeTernary(frame, PArguments.getArgument(frame, 0), PArguments.getArgument(frame, 1), PArguments.getArgument(frame, 2));
+                    }
+                    throw CompilerDirectives.shouldNotReachHere();
                 } else if (ternaryOperator != null) {
                     return callTernaryNode.execute(frame, PArguments.getArgument(frame, 0), PArguments.getArgument(frame, 1), PArguments.getArgument(frame, 2));
                 } else {
@@ -531,7 +534,13 @@ Object execute(Object[] arguments,
                         @Cached HPyAsPythonObjectNode asPythonObjectNode,
                         @Cached HPyAsHandleNode asHandleNode,
                         @Cached GenericInvokeNode invokeNode,
-                        @Cached TransformExceptionToNativeNode transformExceptionToNativeNode) {
+                        @Cached TransformExceptionToNativeNode transformExceptionToNativeNode) throws ArityException {
+
+            // We need to do argument checking at this position because our helper root node that
+            // just dispatches to the appropriate 'LookupAndCallXXXNode' won't do any arguemnt
+            // checking. So it would just crash if there are too few arguments or just ignore if
+            // there are too many.
+            checkArguments(arguments);
 
             GraalHPyContext context = asContextNode.execute(arguments[0]);
             Object[] pythonArguments = PArguments.create(arguments.length - 1);
@@ -549,6 +558,29 @@ Object execute(Object[] arguments,
             }
         }
 
+        private void checkArguments(Object[] arguments) throws ArityException {
+            // note: we always add 1 for the context
+            int min;
+            int max;
+            if (unaryOperator != null) {
+                min = max = 2;
+            } else if (binaryOperator != null) {
+                min = max = 3;
+            } else if (inplaceOperator != null) {
+                min = 3;
+                max = 4;
+            } else if (ternaryOperator != null) {
+                min = max = 4;
+            } else {
+                throw CompilerDirectives.shouldNotReachHere();
+            }
+            if (min > arguments.length || max < arguments.length) {
+                CompilerDirectives.transferToInterpreterAndInvalidate();
+                int expected = min > arguments.length ? min : max;
+                throw ArityException.create(expected, arguments.length);
+            }
+        }
+
         private RootCallTarget getCallTarget(GraalHPyArithmetic receiver, PythonLanguage language) {
             RootCallTarget callTarget = receiver.callTarget;
             if (callTarget == null) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/expression/LookupAndCallInplaceNode.java

@@ -42,6 +42,7 @@
 
 import com.oracle.graal.python.builtins.objects.PNone;
 import com.oracle.graal.python.builtins.objects.PNotImplemented;
+import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.nodes.PNodeWithContext;
 import com.oracle.graal.python.nodes.attributes.LookupInheritedAttributeNode;
 import com.oracle.graal.python.nodes.call.special.CallBinaryMethodNode;
@@ -173,21 +174,19 @@ Object doInplaceOnly(VirtualFrame frame, Object left, Object right, Object z,
     @Specialization(guards = "hasNonInplaceOperator()")
     Object doBinary(VirtualFrame frame, Object left, Object right, Object z,
                     @Cached("create(inplaceOpName)") LookupInheritedAttributeNode getattrInplace) {
-        Object result = PNotImplemented.NOT_IMPLEMENTED;
+        Object result;
         Object inplaceCallable = getattrInplace.execute(left);
-        boolean isBinary = z == PNone.NO_VALUE;
         if (inplaceCallable != PNone.NO_VALUE) {
-            if (isBinary) {
-                result = ensureBinaryCallNode().executeObject(frame, inplaceCallable, left, right);
-            } else {
-                result = ensureTernaryCallNode().execute(frame, inplaceCallable, left, right, z);
-            }
+            // nb.: The only ternary in-place operator is '__ipow__' but according to 'typeobject.c:
+            // slot_nb_inplace_power', this is always called as binary.
+            result = ensureBinaryCallNode().executeObject(frame, inplaceCallable, left, right);
             if (result != PNotImplemented.NOT_IMPLEMENTED) {
                 return result;
             }
         }
 
         // try non-inplace variant
+        boolean isBinary = PGuards.isPNone(z);
         if (isBinary) {
             result = ensureLookupAndCallBinaryNode().executeObject(frame, left, right);
         } else {