Skip to content

Commit 92611d0

Browse files
committed
Address comments
1 parent 734355e commit 92611d0

File tree

2 files changed

+111
-128
lines changed

2 files changed

+111
-128
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/floats/FloatBuiltins.java

Lines changed: 85 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
package com.oracle.graal.python.builtins.objects.floats;
2727

2828
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.OverflowError;
29+
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.TypeError;
2930
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.ValueError;
3031
import static com.oracle.graal.python.nodes.BuiltinNames.J_FLOAT;
3132
import static com.oracle.graal.python.nodes.SpecialMethodNames.J___ABS__;
@@ -91,12 +92,11 @@
9192
import com.oracle.graal.python.builtins.objects.floats.FloatBuiltinsClinicProviders.FormatNodeClinicProviderGen;
9293
import com.oracle.graal.python.builtins.objects.ints.PInt;
9394
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
94-
import com.oracle.graal.python.lib.PyFloatAsDoubleNode;
9595
import com.oracle.graal.python.lib.PyFloatCheckNode;
96-
import com.oracle.graal.python.lib.PyLongAsDoubleNode;
9796
import com.oracle.graal.python.lib.PyLongCheckNode;
9897
import com.oracle.graal.python.lib.PyObjectHashNode;
9998
import com.oracle.graal.python.nodes.ErrorMessages;
99+
import com.oracle.graal.python.nodes.PRaiseNode;
100100
import com.oracle.graal.python.nodes.call.special.LookupAndCallTernaryNode;
101101
import com.oracle.graal.python.nodes.call.special.LookupAndCallVarargsNode;
102102
import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
@@ -111,6 +111,8 @@
111111
import com.oracle.graal.python.nodes.object.InlinedGetClassNode;
112112
import com.oracle.graal.python.nodes.object.InlinedGetClassNode.GetPythonObjectClassNode;
113113
import com.oracle.graal.python.nodes.truffle.PythonArithmeticTypes;
114+
import com.oracle.graal.python.nodes.util.CannotCastException;
115+
import com.oracle.graal.python.nodes.util.CastToJavaDoubleNode;
114116
import com.oracle.graal.python.runtime.exception.PythonErrorType;
115117
import com.oracle.graal.python.runtime.formatting.FloatFormatter;
116118
import com.oracle.graal.python.runtime.formatting.InternalFormat;
@@ -130,7 +132,6 @@
130132
import com.oracle.truffle.api.dsl.TypeSystemReference;
131133
import com.oracle.truffle.api.frame.VirtualFrame;
132134
import com.oracle.truffle.api.interop.InteropLibrary;
133-
import com.oracle.truffle.api.interop.UnsupportedMessageException;
134135
import com.oracle.truffle.api.library.CachedLibrary;
135136
import com.oracle.truffle.api.nodes.Node;
136137
import com.oracle.truffle.api.nodes.UnexpectedResultException;
@@ -297,107 +298,68 @@ static boolean isFloatSubtype(Node inliningTarget, PythonAbstractNativeObject ob
297298
}
298299
}
299300

300-
protected static final int DOUBLE_TYPE = 1;
301-
protected static final int LONG_TYPE = 2;
302-
protected static final int FOREIGN_TYPE = 3;
303-
protected static final int NOT_IMPLEMENTED = 4;
304-
305301
@GenerateUncached
306302
abstract static class ConvertToDoubleCheckNode extends Node {
307-
abstract int execute(Object obj);
303+
abstract boolean execute(Object obj);
308304

309305
@Specialization
310-
static int doDouble(@SuppressWarnings("unused") Double object) {
311-
return DOUBLE_TYPE;
306+
static boolean doDouble(@SuppressWarnings("unused") Double object) {
307+
return true;
312308
}
313309

314310
@Specialization
315-
static int doInt(@SuppressWarnings("unused") Integer object) {
316-
return LONG_TYPE;
311+
static boolean doInt(@SuppressWarnings("unused") Integer object) {
312+
return true;
317313
}
318314

319315
@Specialization
320-
static int doLong(@SuppressWarnings("unused") Long object) {
321-
return LONG_TYPE;
316+
static boolean doLong(@SuppressWarnings("unused") Long object) {
317+
return true;
322318
}
323319

324320
@Specialization
325-
static int doBoolean(@SuppressWarnings("unused") Boolean object) {
326-
return LONG_TYPE;
321+
static boolean doBoolean(@SuppressWarnings("unused") Boolean object) {
322+
return true;
327323
}
328324

329325
@Specialization
330-
static int doString(@SuppressWarnings("unused") TruffleString object) {
331-
return NOT_IMPLEMENTED;
326+
static boolean doString(@SuppressWarnings("unused") TruffleString object) {
327+
return false;
332328
}
333329

334330
@Specialization
335-
static int doPBCT(@SuppressWarnings("unused") PythonBuiltinClassType object) {
336-
return NOT_IMPLEMENTED;
331+
static boolean doPBCT(@SuppressWarnings("unused") PythonBuiltinClassType object) {
332+
return false;
337333
}
338334

339335
@Specialization
340-
static int typeCheck(Object obj,
336+
static boolean typeCheck(Object obj,
341337
@Cached PyFloatCheckNode floatCheckNode,
342338
@Cached PyLongCheckNode longCheckNode,
343339
@Bind("this") Node inliningTarget,
344340
@Cached InlinedGetClassNode getClassNode,
345341
@CachedLibrary(limit = "3") InteropLibrary interopLibrary) {
346-
if (floatCheckNode.execute(inliningTarget, obj)) {
347-
return DOUBLE_TYPE;
348-
}
349-
if (longCheckNode.execute(obj)) {
350-
return LONG_TYPE;
342+
if (floatCheckNode.execute(inliningTarget, obj) || longCheckNode.execute(obj)) {
343+
return true;
351344
}
352345
Object type = getClassNode.execute(inliningTarget, obj);
353346
if (type == PythonBuiltinClassType.ForeignObject) {
354347
if (interopLibrary.fitsInDouble(obj) || interopLibrary.fitsInLong(obj) || interopLibrary.isBoolean(obj)) {
355-
return FOREIGN_TYPE;
348+
return true;
356349
}
357350
}
358-
return NOT_IMPLEMENTED;
351+
return false;
359352
}
360353
}
361354

362-
@ImportStatic(FloatBuiltins.class)
363-
@GenerateUncached
364-
abstract static class ConvertToDoubleNode extends Node {
365-
366-
abstract double execute(VirtualFrame frame, int type, Object obj);
367-
368-
@Specialization(guards = "type == DOUBLE_TYPE")
369-
static double doDouble(VirtualFrame frame, @SuppressWarnings("unused") int type, Object obj,
370-
@Cached PyFloatAsDoubleNode asDoubleNode) {
371-
return asDoubleNode.execute(frame, obj);
372-
}
373-
374-
@Specialization(guards = "type == LONG_TYPE")
375-
static double doLong(@SuppressWarnings("unused") int type, Object obj,
376-
@Cached PyLongAsDoubleNode asDoubleNode) {
377-
Object r = asDoubleNode.execute(obj);
378-
assert r != PNotImplemented.NOT_IMPLEMENTED : "Already been checked in ConvertToDoubleCheckNode";
379-
return (double) r;
380-
}
381-
382-
@Specialization(guards = "type == FOREIGN_TYPE")
383-
static double doForeign(@SuppressWarnings("unused") int type, Object obj,
384-
@CachedLibrary(limit = "3") InteropLibrary interopLibrary) {
385-
try {
386-
if (interopLibrary.fitsInDouble(obj)) {
387-
388-
return interopLibrary.asDouble(obj);
389-
}
390-
if (interopLibrary.fitsInLong(obj)) {
391-
return (double) interopLibrary.asLong(obj);
392-
}
393-
if (interopLibrary.isBoolean(obj)) {
394-
return interopLibrary.asBoolean(obj) ? 1.0 : 0.0;
395-
}
396-
} catch (UnsupportedMessageException e) {
397-
throw CompilerDirectives.shouldNotReachHere(e);
398-
}
399-
400-
throw CompilerDirectives.shouldNotReachHere("Should have been checked in ConvertToDoubleCheckNode");
355+
static double convertToDouble(Object obj,
356+
CastToJavaDoubleNode asDoubleNode,
357+
PRaiseNode raiseNode) {
358+
try {
359+
return asDoubleNode.execute(obj);
360+
} catch (CannotCastException e) {
361+
// This can only happen to values that are expected to be long.
362+
throw raiseNode.raise(TypeError, ErrorMessages.INTEGER_REQUIRED);
401363
}
402364
}
403365

@@ -449,20 +411,20 @@ static Object doDP(VirtualFrame frame, PythonAbstractNativeObject left, double r
449411
}
450412

451413
@Fallback
452-
static Object doGeneric(VirtualFrame frame, Object left, Object right,
414+
Object doGeneric(Object left, Object right,
453415
@Cached ConvertToDoubleCheckNode convertToDoubleCheckNode,
454-
@Cached ConvertToDoubleNode convertToDoubleNode) {
416+
@Cached CastToJavaDoubleNode castToJavaDoubleNode) {
455417
double leftDouble;
456418
double rightDouble;
457-
int leftType = convertToDoubleCheckNode.execute(left);
458-
if (leftType != NOT_IMPLEMENTED) {
459-
leftDouble = convertToDoubleNode.execute(frame, leftType, left);
419+
420+
if (convertToDoubleCheckNode.execute(left)) {
421+
leftDouble = convertToDouble(left, castToJavaDoubleNode, getRaiseNode());
460422
} else {
461423
return PNotImplemented.NOT_IMPLEMENTED;
462424
}
463-
int rightType = convertToDoubleCheckNode.execute(right);
464-
if (rightType != NOT_IMPLEMENTED) {
465-
rightDouble = convertToDoubleNode.execute(frame, rightType, right);
425+
426+
if (convertToDoubleCheckNode.execute(right)) {
427+
rightDouble = convertToDouble(right, castToJavaDoubleNode, getRaiseNode());
466428
} else {
467429
return PNotImplemented.NOT_IMPLEMENTED;
468430
}
@@ -501,20 +463,20 @@ static double doLD(long left, double right) {
501463
}
502464

503465
@Fallback
504-
static Object doGeneric(VirtualFrame frame, Object left, Object right,
466+
Object doGeneric(Object left, Object right,
505467
@Cached ConvertToDoubleCheckNode convertToDoubleCheckNode,
506-
@Cached ConvertToDoubleNode convertToDoubleNode) {
468+
@Cached CastToJavaDoubleNode castToJavaDoubleNode) {
507469
double leftDouble;
508470
double rightDouble;
509-
int leftType = convertToDoubleCheckNode.execute(left);
510-
if (leftType != NOT_IMPLEMENTED) {
511-
leftDouble = convertToDoubleNode.execute(frame, leftType, left);
471+
472+
if (convertToDoubleCheckNode.execute(left)) {
473+
leftDouble = convertToDouble(left, castToJavaDoubleNode, getRaiseNode());
512474
} else {
513475
return PNotImplemented.NOT_IMPLEMENTED;
514476
}
515-
int rightType = convertToDoubleCheckNode.execute(right);
516-
if (rightType != NOT_IMPLEMENTED) {
517-
rightDouble = convertToDoubleNode.execute(frame, rightType, right);
477+
478+
if (convertToDoubleCheckNode.execute(right)) {
479+
rightDouble = convertToDouble(right, castToJavaDoubleNode, getRaiseNode());
518480
} else {
519481
return PNotImplemented.NOT_IMPLEMENTED;
520482
}
@@ -581,20 +543,20 @@ Object doDP(VirtualFrame frame, PythonAbstractNativeObject left, PInt right,
581543
}
582544

583545
@Fallback
584-
static Object doGeneric(VirtualFrame frame, Object left, Object right,
546+
Object doGeneric(Object left, Object right,
585547
@Cached ConvertToDoubleCheckNode convertToDoubleCheckNode,
586-
@Cached ConvertToDoubleNode convertToDoubleNode) {
548+
@Cached CastToJavaDoubleNode castToJavaDoubleNode) {
587549
double leftDouble;
588550
double rightDouble;
589-
int leftType = convertToDoubleCheckNode.execute(left);
590-
if (leftType != NOT_IMPLEMENTED) {
591-
leftDouble = convertToDoubleNode.execute(frame, leftType, left);
551+
552+
if (convertToDoubleCheckNode.execute(left)) {
553+
leftDouble = convertToDouble(left, castToJavaDoubleNode, getRaiseNode());
592554
} else {
593555
return PNotImplemented.NOT_IMPLEMENTED;
594556
}
595-
int rightType = convertToDoubleCheckNode.execute(right);
596-
if (rightType != NOT_IMPLEMENTED) {
597-
rightDouble = convertToDoubleNode.execute(frame, rightType, right);
557+
558+
if (convertToDoubleCheckNode.execute(right)) {
559+
rightDouble = convertToDouble(right, castToJavaDoubleNode, getRaiseNode());
598560
} else {
599561
return PNotImplemented.NOT_IMPLEMENTED;
600562
}
@@ -712,22 +674,22 @@ Object doDPiToComplex(VirtualFrame frame, PInt left, double right, @SuppressWarn
712674
@Specialization
713675
Object doGeneric(VirtualFrame frame, Object left, Object right, Object mod,
714676
@Cached ConvertToDoubleCheckNode convertToDoubleCheckNode,
715-
@Cached ConvertToDoubleNode convertToDoubleNode,
677+
@Cached CastToJavaDoubleNode castToJavaDoubleNode,
716678
@Shared("powCall") @Cached("create(Pow)") LookupAndCallTernaryNode callPow) {
717679
if (!(mod instanceof PNone)) {
718680
throw raise(PythonBuiltinClassType.TypeError, ErrorMessages.POW_3RD_ARG_NOT_ALLOWED_UNLESS_INTEGERS);
719681
}
720682
double leftDouble;
721683
double rightDouble;
722-
int leftType = convertToDoubleCheckNode.execute(left);
723-
if (leftType != NOT_IMPLEMENTED) {
724-
leftDouble = convertToDoubleNode.execute(frame, leftType, left);
684+
685+
if (convertToDoubleCheckNode.execute(left)) {
686+
leftDouble = convertToDouble(left, castToJavaDoubleNode, getRaiseNode());
725687
} else {
726688
return PNotImplemented.NOT_IMPLEMENTED;
727689
}
728-
int rightType = convertToDoubleCheckNode.execute(right);
729-
if (rightType != NOT_IMPLEMENTED) {
730-
rightDouble = convertToDoubleNode.execute(frame, rightType, right);
690+
691+
if (convertToDoubleCheckNode.execute(right)) {
692+
rightDouble = convertToDouble(right, castToJavaDoubleNode, getRaiseNode());
731693
} else {
732694
return PNotImplemented.NOT_IMPLEMENTED;
733695
}
@@ -812,20 +774,20 @@ PTuple doGenericFloat(VirtualFrame frame, Object left, Object right,
812774
}
813775

814776
@Fallback
815-
Object doGeneric(VirtualFrame frame, Object left, Object right,
777+
Object doGeneric(Object left, Object right,
816778
@Cached ConvertToDoubleCheckNode convertToDoubleCheckNode,
817-
@Cached ConvertToDoubleNode convertToDoubleNode) {
779+
@Cached CastToJavaDoubleNode castToJavaDoubleNode) {
818780
double leftDouble;
819781
double rightDouble;
820-
int leftType = convertToDoubleCheckNode.execute(left);
821-
if (leftType != NOT_IMPLEMENTED) {
822-
leftDouble = convertToDoubleNode.execute(frame, leftType, left);
782+
783+
if (convertToDoubleCheckNode.execute(left)) {
784+
leftDouble = convertToDouble(left, castToJavaDoubleNode, getRaiseNode());
823785
} else {
824786
return PNotImplemented.NOT_IMPLEMENTED;
825787
}
826-
int rightType = convertToDoubleCheckNode.execute(right);
827-
if (rightType != NOT_IMPLEMENTED) {
828-
rightDouble = convertToDoubleNode.execute(frame, rightType, right);
788+
789+
if (convertToDoubleCheckNode.execute(right)) {
790+
rightDouble = convertToDouble(right, castToJavaDoubleNode, getRaiseNode());
829791
} else {
830792
return PNotImplemented.NOT_IMPLEMENTED;
831793
}
@@ -1016,20 +978,20 @@ public abstract static class ModNode extends FloatBinaryBuiltinNode {
1016978
}
1017979

1018980
@Fallback
1019-
Object doGeneric(VirtualFrame frame, Object left, Object right,
981+
Object doGeneric(Object left, Object right,
1020982
@Cached ConvertToDoubleCheckNode convertToDoubleCheckNode,
1021-
@Cached ConvertToDoubleNode convertToDoubleNode) {
983+
@Cached CastToJavaDoubleNode castToJavaDoubleNode) {
1022984
double leftDouble;
1023985
double rightDouble;
1024-
int leftType = convertToDoubleCheckNode.execute(left);
1025-
if (leftType != NOT_IMPLEMENTED) {
1026-
leftDouble = convertToDoubleNode.execute(frame, leftType, left);
986+
987+
if (convertToDoubleCheckNode.execute(left)) {
988+
leftDouble = convertToDouble(left, castToJavaDoubleNode, getRaiseNode());
1027989
} else {
1028990
return PNotImplemented.NOT_IMPLEMENTED;
1029991
}
1030-
int rightType = convertToDoubleCheckNode.execute(right);
1031-
if (rightType != NOT_IMPLEMENTED) {
1032-
rightDouble = convertToDoubleNode.execute(frame, rightType, right);
992+
993+
if (convertToDoubleCheckNode.execute(right)) {
994+
rightDouble = convertToDouble(right, castToJavaDoubleNode, getRaiseNode());
1033995
} else {
1034996
return PNotImplemented.NOT_IMPLEMENTED;
1035997
}
@@ -1097,20 +1059,20 @@ Object doDP(VirtualFrame frame, long left, PythonAbstractNativeObject right,
10971059
}
10981060

10991061
@Fallback
1100-
Object doGeneric(VirtualFrame frame, Object left, Object right,
1062+
Object doGeneric(Object left, Object right,
11011063
@Cached ConvertToDoubleCheckNode convertToDoubleCheckNode,
1102-
@Cached ConvertToDoubleNode convertToDoubleNode) {
1064+
@Cached CastToJavaDoubleNode castToJavaDoubleNode) {
11031065
double leftDouble;
11041066
double rightDouble;
1105-
int leftType = convertToDoubleCheckNode.execute(left);
1106-
if (leftType != NOT_IMPLEMENTED) {
1107-
leftDouble = convertToDoubleNode.execute(frame, leftType, left);
1067+
1068+
if (convertToDoubleCheckNode.execute(left)) {
1069+
leftDouble = convertToDouble(left, castToJavaDoubleNode, getRaiseNode());
11081070
} else {
11091071
return PNotImplemented.NOT_IMPLEMENTED;
11101072
}
1111-
int rightType = convertToDoubleCheckNode.execute(right);
1112-
if (rightType != NOT_IMPLEMENTED) {
1113-
rightDouble = convertToDoubleNode.execute(frame, rightType, right);
1073+
1074+
if (convertToDoubleCheckNode.execute(right)) {
1075+
rightDouble = convertToDouble(right, castToJavaDoubleNode, getRaiseNode());
11141076
} else {
11151077
return PNotImplemented.NOT_IMPLEMENTED;
11161078
}

0 commit comments

Comments
 (0)