Skip to content

Commit 0539e0e

Browse files
committed
Fix gates and review comments
1 parent a255a16 commit 0539e0e

14 files changed

+62
-125
lines changed

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -41,7 +41,6 @@
4141
package com.oracle.graal.python.builtins.modules.cext;
4242

4343
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.AttributeError;
44-
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.ImportError;
4544
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.ValueError;
4645
import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Direct;
4746
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.ConstCharPtr;
@@ -366,9 +365,6 @@ static Object doGeneric(TruffleString name, @SuppressWarnings("unused") int noBl
366365
if (object == null) {
367366
// noBlock has no effect anymore since 3.3
368367
object = AbstractImportNode.importModule(trace, T_IMPORT_ALL);
369-
if (object == PNone.NO_VALUE) {
370-
throw raiseNode.get(inliningTarget).raise(ImportError, PY_CAPSULE_IMPORT_S_IS_NOT_VALID, trace);
371-
}
372368
} else {
373369
object = getAttrNode.execute(object, trace);
374370
}

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@
152152
import com.oracle.graal.python.nodes.object.GetClassNode;
153153
import com.oracle.graal.python.nodes.object.GetDictIfExistsNode;
154154
import com.oracle.graal.python.nodes.object.SetDictNode;
155+
import com.oracle.graal.python.nodes.util.CastToTruffleStringNode;
155156
import com.oracle.graal.python.runtime.PythonContext;
156157
import com.oracle.graal.python.runtime.sequence.PSequence;
157158
import com.oracle.graal.python.runtime.sequence.storage.NativeByteSequenceStorage;
@@ -830,14 +831,6 @@ static Object set(PythonAbstractObject object, Object bufferProcs,
830831
@CApiBuiltin(ret = Void, args = {PyTypeObject, PyObject}, call = Ignored)
831832
abstract static class Py_set_PyTypeObject_tp_dict extends CApiBinaryBuiltinNode {
832833

833-
private static TruffleString castKey(Node inliningTarget, CastToTruffleStringNode castNode, Object value) {
834-
try {
835-
return castNode.execute(inliningTarget, value);
836-
} catch (CannotCastException ex) {
837-
throw CompilerDirectives.shouldNotReachHere(ex);
838-
}
839-
}
840-
841834
@Specialization
842835
static Object doTpDict(PythonManagedClass object, Object value,
843836
@Bind("this") Node inliningTarget,
@@ -854,7 +847,7 @@ static Object doTpDict(PythonManagedClass object, Object value,
854847
HashingStorage storage = dict.getDictStorage();
855848
HashingStorageIterator it = getIterator.execute(inliningTarget, storage);
856849
while (itNext.execute(inliningTarget, storage, it)) {
857-
writeAttrNode.execute(object, castKey(inliningTarget, castNode, itKey.execute(inliningTarget, storage, it)), itValue.execute(inliningTarget, storage, it));
850+
writeAttrNode.execute(object, castNode.castKnownString(inliningTarget, itKey.execute(inliningTarget, storage, it)), itValue.execute(inliningTarget, storage, it));
858851
}
859852
PDict existing = getDict.execute(object);
860853
if (existing != null) {

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

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@
5656
import static com.oracle.graal.python.nodes.SpecialAttributeNames.T___NAME__;
5757
import static com.oracle.graal.python.util.PythonUtils.EMPTY_OBJECT_ARRAY;
5858
import static com.oracle.graal.python.util.PythonUtils.TS_ENCODING;
59-
import static com.oracle.truffle.api.CompilerDirectives.shouldNotReachHere;
60-
import static com.oracle.truffle.api.CompilerDirectives.transferToInterpreter;
6159

6260
import com.oracle.graal.python.PythonLanguage;
6361
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApi7BuiltinNode;
@@ -98,7 +96,6 @@
9896
import com.oracle.graal.python.nodes.attributes.LookupAttributeInMRONode;
9997
import com.oracle.graal.python.nodes.attributes.WriteAttributeToDynamicObjectNode;
10098
import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
101-
import com.oracle.graal.python.nodes.util.CannotCastException;
10299
import com.oracle.graal.python.nodes.util.CastToTruffleStringNode;
103100
import com.oracle.graal.python.runtime.PythonContext;
104101
import com.oracle.graal.python.runtime.PythonOptions;
@@ -137,12 +134,7 @@ Object doGeneric(Object type, Object name,
137134
@Bind("this") Node inliningTarget,
138135
@Cached CastToTruffleStringNode castToTruffleStringNode,
139136
@Cached LookupAttributeInMRONode.Dynamic lookupAttributeInMRONode) {
140-
TruffleString key;
141-
try {
142-
key = castToTruffleStringNode.execute(inliningTarget, castToTruffleStringNode);
143-
} catch (CannotCastException e) {
144-
throw shouldNotReachHere();
145-
}
137+
TruffleString key = castToTruffleStringNode.castKnownString(inliningTarget, name);
146138
Object result = lookupAttributeInMRONode.execute(type, key);
147139
if (result == PNone.NO_VALUE) {
148140
return getNativeNull();
@@ -314,12 +306,8 @@ abstract static class PyTruffleType_AddSlot extends CApi7BuiltinNode {
314306
@TruffleBoundary
315307
static int addSlot(Object clazz, PDict tpDict, TruffleString memberName, Object cfunc, int flags, int wrapper, Object memberDoc) {
316308
// create wrapper descriptor
317-
Object wrapperDescriptor = CreateFunctionNode.executeUncached(memberName, cfunc, wrapper, clazz, flags);
318-
if (!(wrapperDescriptor instanceof PythonObject)) {
319-
transferToInterpreter();
320-
throw shouldNotReachHere("Unexpected class of wrapperDescriptor: " + wrapperDescriptor.getClass());
321-
}
322-
WriteAttributeToDynamicObjectNode.getUncached().execute((PythonObject) wrapperDescriptor, SpecialAttributeNames.T___DOC__, memberDoc);
309+
PythonObject wrapperDescriptor = CreateFunctionNode.executeUncached(memberName, cfunc, wrapper, clazz, flags);
310+
WriteAttributeToDynamicObjectNode.getUncached().execute(wrapperDescriptor, SpecialAttributeNames.T___DOC__, memberDoc);
323311

324312
// add wrapper descriptor to tp_dict
325313
PyDictSetDefault.executeUncached(tpDict, memberName, wrapperDescriptor);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CExtNodes.java

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@
6464
import static com.oracle.graal.python.runtime.exception.PythonErrorType.SystemError;
6565
import static com.oracle.graal.python.util.PythonUtils.TS_ENCODING;
6666
import static com.oracle.graal.python.util.PythonUtils.toTruffleStringUncached;
67+
import static com.oracle.truffle.api.CompilerDirectives.shouldNotReachHere;
68+
import static com.oracle.truffle.api.CompilerDirectives.transferToInterpreter;
6769

6870
import java.util.regex.Matcher;
6971
import java.util.regex.Pattern;
@@ -212,7 +214,7 @@ public abstract static class SubtypeNew extends Node {
212214
* tget the <code>typename_subtype_new</code> function
213215
*/
214216
protected NativeCAPISymbol getFunction() {
215-
throw CompilerDirectives.shouldNotReachHere();
217+
throw shouldNotReachHere();
216218
}
217219

218220
protected abstract Object execute(Object object, Object arg);
@@ -238,7 +240,7 @@ Object callNativeConstructor(Object object, Object arg,
238240
Object result = interopLibrary.execute(importCAPISymbolNode.execute(inliningTarget, cApiContext, getFunction()), toSulongNode.execute(object), arg);
239241
return toJavaNode.execute(result);
240242
} catch (UnsupportedMessageException | UnsupportedTypeException | ArityException e) {
241-
throw CompilerDirectives.shouldNotReachHere("C subtype_new function failed", e);
243+
throw shouldNotReachHere("C subtype_new function failed", e);
242244
}
243245
}
244246
}
@@ -839,7 +841,7 @@ static Object doWithoutContext(NativeCAPISymbol name, Object[] args,
839841
return ensureTruffleStringNode.execute(inliningTarget, interopLibrary.execute(importCExtSymbolNode.execute(inliningTarget, cApiContext, name), args));
840842
} catch (UnsupportedTypeException | ArityException | UnsupportedMessageException e) {
841843
// consider these exceptions to be fatal internal errors
842-
throw CompilerDirectives.shouldNotReachHere(e);
844+
throw shouldNotReachHere(e);
843845
}
844846
}
845847

@@ -1126,7 +1128,7 @@ static Object doNativeObject(Object object, long value,
11261128
long refCount = CApiTransitions.readNativeRefCount(pointer);
11271129
CApiTransitions.writeNativeRefCount(pointer, refCount + value);
11281130
} catch (UnsupportedMessageException e) {
1129-
throw CompilerDirectives.shouldNotReachHere(e);
1131+
throw shouldNotReachHere(e);
11301132
}
11311133
}
11321134
return object;
@@ -1219,7 +1221,7 @@ static Object resolveGeneric(Node inliningTarget, Object pointerObject,
12191221
try {
12201222
pointer = lib.asPointer(pointerObject);
12211223
} catch (UnsupportedMessageException e) {
1222-
throw CompilerDirectives.shouldNotReachHere(e);
1224+
throw shouldNotReachHere(e);
12231225
}
12241226
lookup = CApiTransitions.lookupNative(pointer);
12251227
if (lookup != null) {
@@ -1427,7 +1429,7 @@ Object doGeneric(TruffleString f, Object vaList) {
14271429
// That should really not happen because we created the unicode
14281430
// object with FromCharPointerNode which guarantees to return a
14291431
// String/PString.
1430-
throw CompilerDirectives.shouldNotReachHere();
1432+
throw shouldNotReachHere();
14311433
}
14321434
vaArgIdx++;
14331435
valid = true;
@@ -1516,7 +1518,7 @@ private static int getAndCastToInt(InteropLibrary lib, PRaiseNode raiseNode, Obj
15161518
try {
15171519
return lib.asInt(value);
15181520
} catch (UnsupportedMessageException e) {
1519-
throw CompilerDirectives.shouldNotReachHere();
1521+
throw shouldNotReachHere();
15201522
}
15211523
}
15221524
if (!lib.isPointer(value)) {
@@ -1526,7 +1528,7 @@ private static int getAndCastToInt(InteropLibrary lib, PRaiseNode raiseNode, Obj
15261528
try {
15271529
return (int) lib.asPointer(value);
15281530
} catch (UnsupportedMessageException e) {
1529-
throw CompilerDirectives.shouldNotReachHere();
1531+
throw shouldNotReachHere();
15301532
}
15311533
}
15321534
throw raiseNode.raise(PythonBuiltinClassType.SystemError, ErrorMessages.P_OBJ_CANT_BE_INTEPRETED_AS_INTEGER, value);
@@ -1541,7 +1543,7 @@ private static long castToLong(InteropLibrary lib, PRaiseNode raiseNode, Object
15411543
try {
15421544
return lib.asLong(value);
15431545
} catch (UnsupportedMessageException e) {
1544-
throw CompilerDirectives.shouldNotReachHere();
1546+
throw shouldNotReachHere();
15451547
}
15461548
}
15471549
if (!lib.isPointer(value)) {
@@ -1551,7 +1553,7 @@ private static long castToLong(InteropLibrary lib, PRaiseNode raiseNode, Object
15511553
try {
15521554
return lib.asPointer(value);
15531555
} catch (UnsupportedMessageException e) {
1554-
throw CompilerDirectives.shouldNotReachHere();
1556+
throw shouldNotReachHere();
15551557
}
15561558
}
15571559
throw raiseNode.raise(PythonBuiltinClassType.SystemError, ErrorMessages.P_OBJ_CANT_BE_INTEPRETED_AS_INTEGER, value);
@@ -1681,7 +1683,7 @@ static Object doGeneric(CApiContext capiContext, ModuleSpec moduleSpec, Object m
16811683
ErrorMessages.CREATION_FAILD_WITHOUT_EXCEPTION, ErrorMessages.CREATION_RAISED_EXCEPTION);
16821684
module = toJavaNode.execute(result);
16831685
} catch (UnsupportedTypeException | ArityException | UnsupportedMessageException e) {
1684-
throw CompilerDirectives.shouldNotReachHere(e);
1686+
throw shouldNotReachHere(e);
16851687
}
16861688

16871689
/*
@@ -1801,7 +1803,7 @@ static int doGeneric(CApiContext capiContext, PythonModule module, Object module
18011803
}
18021804
}
18031805
} catch (UnsupportedMessageException | UnsupportedTypeException | ArityException e) {
1804-
throw CompilerDirectives.shouldNotReachHere();
1806+
throw shouldNotReachHere();
18051807
}
18061808

18071809
return 0;
@@ -1940,23 +1942,23 @@ public abstract static class CreateFunctionNode extends Node {
19401942

19411943
private static final TruffleLogger LOGGER = CApiContext.getLogger(CreateFunctionNode.class);
19421944

1943-
public static Object executeUncached(TruffleString name, Object callable, int wrapper, Object type, Object flags) {
1945+
public static PythonObject executeUncached(TruffleString name, Object callable, int wrapper, Object type, Object flags) {
19441946
return CreateFunctionNodeGen.getUncached().execute(null, name, callable, wrapper, type, flags);
19451947
}
19461948

1947-
public abstract Object execute(Node inliningTarget, TruffleString name, Object callable, int wrapper, Object type, Object flags);
1949+
public abstract PythonObject execute(Node inliningTarget, TruffleString name, Object callable, int wrapper, Object type, Object flags);
19481950

19491951
@Specialization(guards = "!isNoValue(type)")
19501952
@TruffleBoundary
1951-
static Object doPythonCallable(TruffleString name, PythonNativeWrapper callable, int signature, Object type, int flags) {
1953+
static PythonObject doPythonCallable(TruffleString name, PythonNativeWrapper callable, int signature, Object type, int flags) {
19521954
// This can happen if a native type inherits slots from a managed type. Therefore,
19531955
// something like 'base->tp_new' will be a wrapper of the managed '__new__'. So, in this
19541956
// case, we assume that the object is already callable.
19551957
Object managedCallable = callable.getDelegate();
19561958
PythonContext context = PythonContext.get(null);
19571959
PythonLanguage language = context.getLanguage();
19581960
PBuiltinFunction function = PExternalFunctionWrapper.createWrapperFunction(name, managedCallable, type, flags, signature, language, context.factory(), false);
1959-
return function != null ? function : managedCallable;
1961+
return function != null ? function : castToPythonObject(managedCallable);
19601962
}
19611963

19621964
@Specialization
@@ -1971,7 +1973,7 @@ static PBuiltinFunction doPyCFunctionWrapper(TruffleString name, PyCFunctionWrap
19711973

19721974
@Specialization(guards = {"!isNativeWrapper(callable)"})
19731975
@TruffleBoundary
1974-
static Object doNativeCallableWithWrapper(TruffleString name, Object callable, int signature, Object type, int flags,
1976+
static PythonObject doNativeCallableWithWrapper(TruffleString name, Object callable, int signature, Object type, int flags,
19751977
@CachedLibrary(limit = "3") InteropLibrary lib) {
19761978
/*
19771979
* This can happen if a native type inherits slots from a managed type. For example, if
@@ -1990,7 +1992,15 @@ static Object doNativeCallableWithWrapper(TruffleString name, Object callable, i
19901992
}
19911993
PythonLanguage language = context.getLanguage();
19921994
PBuiltinFunction function = PExternalFunctionWrapper.createWrapperFunction(name, resolvedCallable, type, flags, signature, language, context.factory(), doArgAndResultConversion);
1993-
return function != null ? function : resolvedCallable;
1995+
return function != null ? function : castToPythonObject(resolvedCallable);
1996+
}
1997+
1998+
private static PythonObject castToPythonObject(Object callable) {
1999+
if (callable instanceof PythonObject pythonObject) {
2000+
return pythonObject;
2001+
}
2002+
transferToInterpreter();
2003+
throw shouldNotReachHere("Unexpected class of callable: " + callable.getClass());
19942004
}
19952005

19962006
@TruffleBoundary
@@ -2000,7 +2010,7 @@ public static Object resolveClosurePointer(PythonContext context, Object callabl
20002010
try {
20012011
pointer = lib.asPointer(callable);
20022012
} catch (UnsupportedMessageException e) {
2003-
throw CompilerDirectives.shouldNotReachHere(e);
2013+
throw shouldNotReachHere(e);
20042014
}
20052015
Object delegate = context.getCApiContext().getClosureDelegate(pointer);
20062016
if (delegate != null) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/superobject/SuperBuiltins.java

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,7 @@
4949
import static com.oracle.graal.python.nodes.SpecialMethodNames.J___GET__;
5050
import static com.oracle.graal.python.nodes.SpecialMethodNames.J___INIT__;
5151
import static com.oracle.graal.python.nodes.SpecialMethodNames.J___REPR__;
52-
import static com.oracle.graal.python.nodes.truffle.TruffleStringMigrationHelpers.isJavaString;
5352
import static com.oracle.graal.python.util.PythonUtils.TS_ENCODING;
54-
import static com.oracle.graal.python.util.PythonUtils.toTruffleStringUncached;
55-
import static com.oracle.truffle.api.CompilerDirectives.shouldNotReachHere;
5653

5754
import java.util.List;
5855

@@ -68,7 +65,7 @@
6865
import com.oracle.graal.python.builtins.objects.function.PKeyword;
6966
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltins;
7067
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltinsFactory;
71-
import com.oracle.graal.python.builtins.objects.str.PString;
68+
import com.oracle.graal.python.builtins.objects.str.StringNodes.CastToTruffleStringCheckedNode;
7269
import com.oracle.graal.python.builtins.objects.str.StringUtils.SimpleTruffleStringFormatNode;
7370
import com.oracle.graal.python.builtins.objects.superobject.SuperBuiltinsFactory.GetObjectNodeGen;
7471
import com.oracle.graal.python.builtins.objects.superobject.SuperBuiltinsFactory.GetTypeNodeGen;
@@ -439,7 +436,8 @@ private Object genericGetAttr(VirtualFrame frame, Object object, Object attr) {
439436
Object get(VirtualFrame frame, SuperObject self, Object attr,
440437
@Bind("this") Node inliningTarget,
441438
@Cached TruffleString.EqualNode equalNode,
442-
@Cached GetObjectTypeNode getObjectType) {
439+
@Cached GetObjectTypeNode getObjectType,
440+
@Cached CastToTruffleStringCheckedNode castToTruffleStringNode) {
443441
Object startType = getObjectType.execute(inliningTarget, self);
444442
if (startType == null) {
445443
return genericGetAttr(frame, self, attr);
@@ -449,16 +447,7 @@ Object get(VirtualFrame frame, SuperObject self, Object attr,
449447
* We want __class__ to return the class of the super object (i.e. super, or a
450448
* subclass), not the class of su->obj.
451449
*/
452-
TruffleString stringAttr;
453-
if (attr instanceof PString) {
454-
stringAttr = ((PString) attr).getValueUncached();
455-
} else if (isJavaString(attr)) {
456-
stringAttr = toTruffleStringUncached((String) attr);
457-
} else if (attr instanceof TruffleString) {
458-
stringAttr = (TruffleString) attr;
459-
} else {
460-
throw shouldNotReachHere();
461-
}
450+
TruffleString stringAttr = castToTruffleStringNode.cast(inliningTarget, attr, ErrorMessages.ATTR_NAME_MUST_BE_STRING, attr);
462451
if (equalNode.execute(stringAttr, T___CLASS__, TS_ENCODING)) {
463452
return genericGetAttr(frame, self, T___CLASS__);
464453
}

0 commit comments

Comments
 (0)