Skip to content

Commit a676629

Browse files
cosminbascacosminbasca
committed
CopyKeywordsNode: fix rewriteOn exception (typo)
- missing TypeError case
1 parent 55c7f76 commit a676629

File tree

3 files changed

+20
-47
lines changed

3 files changed

+20
-47
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_type.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,11 @@ def testfunction(self):
208208

209209
cached = functools.lru_cache(1)(testfunction)
210210

211-
assert not type(repr).__flags__ & TPFLAGS_METHOD_DESCRIPTOR
212-
assert type(list.append).__flags__ & TPFLAGS_METHOD_DESCRIPTOR
213-
assert type(list.__add__).__flags__ & TPFLAGS_METHOD_DESCRIPTOR
214-
assert type(testfunction).__flags__ & TPFLAGS_METHOD_DESCRIPTOR
215-
assert type(cached).__flags__ & TPFLAGS_METHOD_DESCRIPTOR
211+
assert not type(repr).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, "masked __flags__ = {}, expected {}".format(type(repr).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, 0)
212+
assert type(list.append).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, "masked __flags__ = {}, expected {}".format(type(repr).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, TPFLAGS_METHOD_DESCRIPTOR)
213+
assert type(list.__add__).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, "masked __flags__ = {}, expected {}".format(type(repr).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, TPFLAGS_METHOD_DESCRIPTOR)
214+
assert type(testfunction).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, "masked __flags__ = {}, expected {}".format(type(repr).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, TPFLAGS_METHOD_DESCRIPTOR)
215+
assert type(cached).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, "masked __flags__ = {}, expected {}".format(type(repr).__flags__ & TPFLAGS_METHOD_DESCRIPTOR, TPFLAGS_METHOD_DESCRIPTOR)
216216

217217
class MyInt(int):
218218
pass
@@ -251,4 +251,4 @@ class MyDict(dict):
251251
({"1":1, "2": 2}, TPFLAGS_DICT_SUBCLASS),
252252
(MyDict({"1":1, "2": 2}), TPFLAGS_DICT_SUBCLASS),
253253
]:
254-
assert type(x).__flags__ & flag
254+
assert type(x).__flags__ & flag, "masked __flags__ = {}, expected {}".format(type(x).__flags__ & flag, flag)

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/CopyKeywordsNode.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import com.oracle.graal.python.nodes.control.GetIteratorExpressionNode;
5656
import com.oracle.graal.python.nodes.control.GetNextNode;
5757
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
58+
import com.oracle.graal.python.nodes.util.CannotCastException;
5859
import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
5960
import com.oracle.graal.python.runtime.PythonOptions;
6061
import com.oracle.graal.python.runtime.exception.PException;
@@ -101,7 +102,7 @@ public CopyKeywordsState execute(Object key, CopyKeywordsState state) {
101102

102103
@GenerateUncached
103104
abstract static class AddKeywordNode extends AbstractKeywordsNode {
104-
@Specialization(rewriteOn = ClassCastException.class, limit = "getCallSiteInlineCacheMaxDepth()")
105+
@Specialization(rewriteOn = CannotCastException.class, limit = "getCallSiteInlineCacheMaxDepth()")
105106
public CopyKeywordsState add(Object key, CopyKeywordsState state,
106107
@Cached CastToJavaStringNode castToJavaStringNode,
107108
@CachedLibrary(value = "state.getHashingStorage()") HashingStorageLibrary lib) {
@@ -118,7 +119,7 @@ public CopyKeywordsState addExc(Object key, CopyKeywordsState state,
118119
try {
119120
Object value = lib.getItem(state.hashingStorage, key);
120121
state.addKeyword(castToJavaStringNode.execute(key), value);
121-
} catch (ClassCastException e) {
122+
} catch (CannotCastException e) {
122123
throw raiseNode.raise(TypeError, ErrorMessages.MUST_BE_STRINGS, "keywords");
123124
}
124125
return state;
@@ -151,6 +152,7 @@ void doDict(PArguments.ThreadState state, PDict starargs, PKeyword[] keywords,
151152
@Cached CastToJavaStringNode castToJavaStringNode,
152153
@Cached IsBuiltinClassProfile errorProfile,
153154
@CachedLibrary(limit = "getCallSiteInlineCacheMaxDepth()") PythonObjectLibrary pol,
155+
@Cached PRaiseNode raiseNode,
154156
@SuppressWarnings("unused") @Cached IsBuiltinClassProfile classProfile) {
155157
Object iter = getIteratorNode.executeWithGlobalState(starargs);
156158
int i = 0;
@@ -163,6 +165,8 @@ void doDict(PArguments.ThreadState state, PDict starargs, PKeyword[] keywords,
163165
} catch (PException e) {
164166
e.expectStopIteration(errorProfile);
165167
break;
168+
} catch (CannotCastException e) {
169+
throw raiseNode.raise(TypeError, ErrorMessages.MUST_BE_STRINGS, "keywords");
166170
}
167171
}
168172
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/argument/keywords/ExecuteKeywordStarargsNode.java

Lines changed: 8 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,13 @@
4040
*/
4141
package com.oracle.graal.python.nodes.argument.keywords;
4242

43-
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.TypeError;
44-
4543
import com.oracle.graal.python.builtins.objects.common.EmptyStorage;
4644
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
4745
import com.oracle.graal.python.builtins.objects.common.KeywordsStorage;
4846
import com.oracle.graal.python.builtins.objects.dict.PDict;
4947
import com.oracle.graal.python.builtins.objects.function.PKeyword;
50-
import com.oracle.graal.python.builtins.objects.str.PString;
51-
import com.oracle.graal.python.nodes.ErrorMessages;
5248
import com.oracle.graal.python.nodes.PGuards;
5349
import com.oracle.graal.python.nodes.PNodeWithContext;
54-
import com.oracle.graal.python.nodes.PRaiseNode;
5550
import com.oracle.graal.python.nodes.argument.keywords.ExecuteKeywordStarargsNodeGen.ExpandKeywordStarargsNodeGen;
5651
import com.oracle.graal.python.nodes.expression.ExpressionNode;
5752
import com.oracle.graal.python.runtime.PythonOptions;
@@ -62,9 +57,7 @@
6257
import com.oracle.truffle.api.dsl.Specialization;
6358
import com.oracle.truffle.api.frame.VirtualFrame;
6459
import com.oracle.truffle.api.library.CachedLibrary;
65-
import com.oracle.truffle.api.nodes.ControlFlowException;
6660
import com.oracle.truffle.api.nodes.Node;
67-
import com.oracle.truffle.api.profiles.BranchProfile;
6861

6962
@NodeChild(value = "starargs", type = ExpressionNode.class)
7063
public abstract class ExecuteKeywordStarargsNode extends PNodeWithContext {
@@ -102,26 +95,17 @@ static PKeyword[] doEmptyStorage(@SuppressWarnings("unused") PDict starargs) {
10295
static PKeyword[] doDictCached(PDict starargs,
10396
@Cached CopyKeywordsNode copyKeywordsNode,
10497
@SuppressWarnings("unused") @CachedLibrary("starargs.getDictStorage()") HashingStorageLibrary lib,
105-
@Cached("len(lib, starargs)") int cachedLen,
106-
@Cached PRaiseNode raise,
107-
@Cached BranchProfile errorProfile) {
108-
try {
109-
PKeyword[] keywords = new PKeyword[cachedLen];
110-
copyKeywordsNode.executeWithoutState(starargs, keywords);
111-
return keywords;
112-
} catch (KeywordNotStringException e) {
113-
errorProfile.enter();
114-
throw raise.raise(TypeError, ErrorMessages.MUST_BE_STRINGS, "keywords");
115-
}
98+
@Cached("len(lib, starargs)") int cachedLen) {
99+
PKeyword[] keywords = new PKeyword[cachedLen];
100+
copyKeywordsNode.executeWithoutState(starargs, keywords);
101+
return keywords;
116102
}
117103

118104
@Specialization(replaces = "doDictCached", limit = "1")
119105
static PKeyword[] doDict(PDict starargs,
120106
@Cached CopyKeywordsNode copyKeywordsNode,
121-
@CachedLibrary("starargs.getDictStorage()") HashingStorageLibrary lib,
122-
@Cached PRaiseNode raise,
123-
@Cached BranchProfile errorProfile) {
124-
return doDictCached(starargs, copyKeywordsNode, lib, len(lib, starargs), raise, errorProfile);
107+
@CachedLibrary("starargs.getDictStorage()") HashingStorageLibrary lib) {
108+
return doDictCached(starargs, copyKeywordsNode, lib, len(lib, starargs));
125109
}
126110

127111
@Specialization(guards = "!isDict(object)")
@@ -132,30 +116,19 @@ static PKeyword[] doNonMapping(@SuppressWarnings("unused") Object object) {
132116
@Specialization(replaces = {"doKeywordsArray", "doKeywordsStorage", "doEmptyStorage", "doDictCached", "doDict", "doNonMapping"})
133117
static PKeyword[] doGeneric(Object kwargs,
134118
@Cached CopyKeywordsNode copyKeywordsNode,
135-
@CachedLibrary(limit = "1") HashingStorageLibrary lib,
136-
@Cached PRaiseNode raise,
137-
@Cached BranchProfile errorProfile) {
119+
@CachedLibrary(limit = "1") HashingStorageLibrary lib) {
138120
if (kwargs instanceof PDict) {
139121
PDict d = (PDict) kwargs;
140122
if (isKeywordsStorage(d)) {
141123
return doKeywordsStorage(d);
142124
} else if (isEmptyStorage(d)) {
143125
return doEmptyStorage(d);
144126
}
145-
return doDict(d, copyKeywordsNode, lib, raise, errorProfile);
127+
return doDict(d, copyKeywordsNode, lib);
146128
}
147129
return PKeyword.EMPTY_KEYWORDS;
148130
}
149131

150-
private static String castToString(Object key) throws KeywordNotStringException {
151-
if (key instanceof String) {
152-
return (String) key;
153-
} else if (key instanceof PString) {
154-
return ((PString) key).getValue();
155-
}
156-
throw new KeywordNotStringException();
157-
}
158-
159132
static boolean isKeywordsStorage(PDict dict) {
160133
return dict.getDictStorage() instanceof KeywordsStorage;
161134
}
@@ -168,10 +141,6 @@ static int len(HashingStorageLibrary lib, PDict dict) {
168141
return lib.length(dict.getDictStorage());
169142
}
170143

171-
private static final class KeywordNotStringException extends ControlFlowException {
172-
private static final long serialVersionUID = 1L;
173-
}
174-
175144
public static ExpandKeywordStarargsNode create() {
176145
return ExpandKeywordStarargsNodeGen.create();
177146
}

0 commit comments

Comments
 (0)