Skip to content

Commit b97ac87

Browse files
msimacekmsimacek
committed
Improve error handling when merging keyword arguments
1 parent f6a4642 commit b97ac87

File tree

6 files changed

+81
-29
lines changed

6 files changed

+81
-29
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/str/StringNodes.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,8 @@ static NativeCharSequence getNativeCharSequence(PString self) {
244244
}
245245
}
246246

247-
@ImportStatic(PGuards.class)
248-
public abstract static class CastToJavaStringCheckedNode extends Node {
247+
@GenerateUncached
248+
public abstract static class CastToJavaStringCheckedNode extends PNodeWithContext {
249249
public final String cast(Object object, String errMsgFormat, Object... errMsgArgs) {
250250
return execute(object, errMsgFormat, errMsgArgs);
251251
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyObjectFunctionStr.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package com.oracle.graal.python.lib;
2+
3+
import static com.oracle.graal.python.nodes.SpecialAttributeNames.__MODULE__;
4+
import static com.oracle.graal.python.nodes.SpecialAttributeNames.__QUALNAME__;
5+
6+
import com.oracle.graal.python.builtins.objects.PNone;
7+
import com.oracle.graal.python.builtins.objects.str.PString;
8+
import com.oracle.graal.python.nodes.PNodeWithContext;
9+
import com.oracle.truffle.api.dsl.Cached;
10+
import com.oracle.truffle.api.dsl.GenerateUncached;
11+
import com.oracle.truffle.api.dsl.Specialization;
12+
import com.oracle.truffle.api.frame.Frame;
13+
import com.oracle.truffle.api.frame.VirtualFrame;
14+
15+
/**
16+
* Obtains a string representation of a function for error reporting. Equivalent of CPython's
17+
* {@code _PyObject_FunctionStr}.
18+
*/
19+
@GenerateUncached
20+
public abstract class PyObjectFunctionStr extends PNodeWithContext {
21+
public abstract String execute(Frame frame, Object function);
22+
23+
@Specialization
24+
String str(VirtualFrame frame, Object function,
25+
@Cached PyObjectLookupAttr lookupQualname,
26+
@Cached PyObjectLookupAttr lookupModule,
27+
@Cached PyObjectStrAsJavaStringNode asStr) {
28+
Object qualname = lookupQualname.execute(frame, function, __QUALNAME__);
29+
if (qualname == PNone.NO_VALUE) {
30+
return asStr.execute(function);
31+
}
32+
Object module = lookupModule.execute(frame, function, __MODULE__);
33+
if (!(module instanceof PNone)) {
34+
String moduleStr = asStr.execute(frame, module);
35+
if (!"builtins".equals(moduleStr)) {
36+
return PString.cat(moduleStr, ".", asStr.execute(frame, qualname), "()");
37+
}
38+
}
39+
return PString.cat(asStr.execute(frame, qualname), "()");
40+
}
41+
42+
public static PyObjectFunctionStr create() {
43+
return PyObjectFunctionStrNodeGen.create();
44+
}
45+
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/ErrorMessages.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public abstract class ErrorMessages {
4646
public static final String S_EXPECTED_SD_ARGS_GOT_D = "%s expected %s%d argument%s, got %d";
4747
public static final String UNPACKED_TUPLE_SHOULD_HAVE_D_ELEMS = "unpacked tuple should have %s%d element%s, but has %d";
4848
public static final String ARG_AFTER_MUST_BE_ITERABLE = "argument after * must be an iterable, not %p";
49-
public static final String ARG_AFTER_MUST_BE_MAPPING = "%s() argument after ** must be a mapping, not %p";
49+
public static final String ARG_AFTER_MUST_BE_MAPPING = "%s argument after ** must be a mapping, not %p";
5050
public static final String ARG_CONVERTED_NOT_EXECUTABLE = "argument converted is not executable";
5151
public static final String ARG_CANNOT_BE_NEGATIVE = "%s argument cannot be negative";
5252
public static final String ARG_D_MUST_BE_S = "%s arg %d must be a %s";
@@ -299,7 +299,8 @@ public abstract class ErrorMessages {
299299
public static final String GETATTR_ATTRIBUTE_NAME_MUST_BE_STRING = "getattr(): attribute name must be string";
300300
public static final String GETTING_THER_SOURCE_NOT_SUPPORTED_FOR_P = "getting the source is not supported for '%p'";
301301
public static final String GLOBALS_MUST_BE_DICT = "%s() globals must be a dict, not %p";
302-
public static final String GOT_MULTIPLE_VALUES_FOR_ARG = "%s() got multiple values for keyword argument '%s'";
302+
public static final String GOT_MULTIPLE_VALUES_FOR_ARG = "%s() got multiple values for argument '%s'";
303+
public static final String GOT_MULTIPLE_VALUES_FOR_KEYWORD_ARG = "%s got multiple values for keyword argument '%s'";
303304
public static final String GOT_SOME_POS_ONLY_ARGS_PASSED_AS_KEYWORD = "%s() got some positional-only arguments passed as keyword arguments: '%s'";
304305
public static final String GOT_UNEXPECTED_KEYWORD_ARG = "%s() got an unexpected keyword argument '%s'";
305306
public static final String HANDLER_MUST_BE_CALLABLE = "handler must be callable";
@@ -396,8 +397,7 @@ public abstract class ErrorMessages {
396397
public static final String ISSUBCLASS_MUST_BE_CLASS_OR_TUPLE = "issubclass() arg 2 must be a class or tuple of classes";
397398
public static final String ITER_V_MUST_BE_CALLABLE = "iter(v, w): v must be callable";
398399
public static final String KEYWORD_NAMES_MUST_BE_STR_GOT_P = "keyword names must be str, get %p";
399-
public static final String KEYWORDS_S_MUST_BE_STRINGS = "%s() keywords must be strings";
400-
public static final String KEYWORDS_MUST_BE_STRINGS = "keywords must be strings";
400+
public static final String KEYWORDS_S_MUST_BE_STRINGS = "%s keywords must be strings";
401401
public static final String KLASS_ARG_IS_NOT_HOST_OBJ = "klass argument '%p' is not a host object";
402402
public static final String LAZY_INITIALIZATION_FAILED = "lazy initialization of type %s failed";
403403
public static final String LEFT_BRACKET_WO_RIGHT_BRACKET_IN_ARG = "')' without '(' in argument parsing";

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@
6363
import com.oracle.truffle.api.dsl.Cached;
6464
import com.oracle.truffle.api.dsl.Cached.Shared;
6565
import com.oracle.truffle.api.dsl.Fallback;
66+
import com.oracle.truffle.api.dsl.GenerateUncached;
6667
import com.oracle.truffle.api.dsl.Specialization;
68+
import com.oracle.truffle.api.frame.Frame;
6769
import com.oracle.truffle.api.frame.VirtualFrame;
6870
import com.oracle.truffle.api.library.CachedLibrary;
6971
import com.oracle.truffle.api.nodes.ExplodeLoop;
@@ -99,19 +101,20 @@ protected ConcatDictToStorageNode[] createConcatNodes() {
99101
return nodes;
100102
}
101103

102-
protected abstract static class ConcatDictToStorageNode extends PNodeWithContext {
103-
public abstract HashingStorage execute(VirtualFrame frame, HashingStorage dest, Object other);
104+
@GenerateUncached
105+
public abstract static class ConcatDictToStorageNode extends PNodeWithContext {
106+
public abstract HashingStorage execute(Frame frame, HashingStorage dest, Object other);
104107

105108
@Specialization(guards = "hasBuiltinIter(other, getClassNode, lookupIter)", limit = "1")
106-
HashingStorage doBuiltinDictEmptyDest(@SuppressWarnings("unused") EmptyStorage dest, PDict other,
109+
static HashingStorage doBuiltinDictEmptyDest(@SuppressWarnings("unused") EmptyStorage dest, PDict other,
107110
@SuppressWarnings("unused") @Shared("getClassNode") @Cached GetClassNode getClassNode,
108111
@SuppressWarnings("unused") @Shared("lookupIter") @Cached(parameters = "Iter") LookupCallableSlotInMRONode lookupIter,
109112
@Shared("hlib") @CachedLibrary(limit = "3") HashingStorageLibrary hlib) {
110113
return hlib.copy(other.getDictStorage());
111114
}
112115

113116
@Specialization(guards = "hasBuiltinIter(other, getClassNode, lookupIter)", limit = "1")
114-
HashingStorage doBuiltinDict(VirtualFrame frame, HashingStorage dest, PDict other,
117+
static HashingStorage doBuiltinDict(VirtualFrame frame, HashingStorage dest, PDict other,
115118
@SuppressWarnings("unused") @Shared("getClassNode") @Cached GetClassNode getClassNode,
116119
@SuppressWarnings("unused") @Shared("lookupIter") @Cached(parameters = "Iter") LookupCallableSlotInMRONode lookupIter,
117120
@Shared("hlib") @CachedLibrary(limit = "3") HashingStorageLibrary hlib,
@@ -131,7 +134,7 @@ HashingStorage doBuiltinDict(VirtualFrame frame, HashingStorage dest, PDict othe
131134
}
132135

133136
@Fallback
134-
HashingStorage doMapping(VirtualFrame frame, HashingStorage dest, Object other,
137+
static HashingStorage doMapping(VirtualFrame frame, HashingStorage dest, Object other,
135138
@Shared("hlib") @CachedLibrary(limit = "3") HashingStorageLibrary hlib,
136139
@Shared("hasFrame") @Cached ConditionProfile hasFrame,
137140
@Shared("sameKeyProfile") @Cached BranchProfile sameKeyProfile,
@@ -167,5 +170,13 @@ HashingStorage doMapping(VirtualFrame frame, HashingStorage dest, Object other,
167170
protected static boolean hasBuiltinIter(PDict dict, GetClassNode getClassNode, LookupCallableSlotInMRONode lookupIter) {
168171
return PGuards.isBuiltinDict(dict) || lookupIter.execute(getClassNode.execute(dict)) == BuiltinMethodDescriptors.DICT_ITER;
169172
}
173+
174+
public static ConcatDictToStorageNode create() {
175+
return ConcatKeywordsNodeGen.ConcatDictToStorageNodeGen.create();
176+
}
177+
178+
public static ConcatDictToStorageNode getUncached() {
179+
return ConcatKeywordsNodeGen.ConcatDictToStorageNodeGen.getUncached();
180+
}
170181
}
171182
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/builtins/ListNodes.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,10 @@ public static ConstructListNode getUncached() {
149149
}
150150
}
151151

152-
@ImportStatic(PGuards.class)
152+
@GenerateUncached
153153
public abstract static class FastConstructListNode extends PNodeWithContext {
154154

155-
@Child private ConstructListNode constructListNode;
156-
157-
public abstract PSequence execute(VirtualFrame frame, Object value);
155+
public abstract PSequence execute(Frame frame, Object value);
158156

159157
@Specialization(guards = "cannotBeOverridden(value, getClassNode)", limit = "1")
160158
protected static PSequence doPList(PSequence value,
@@ -163,11 +161,8 @@ protected static PSequence doPList(PSequence value,
163161
}
164162

165163
@Fallback
166-
protected PSequence doGeneric(VirtualFrame frame, Object value) {
167-
if (constructListNode == null) {
168-
CompilerDirectives.transferToInterpreterAndInvalidate();
169-
constructListNode = insert(ConstructListNode.create());
170-
}
164+
protected PSequence doGeneric(VirtualFrame frame, Object value,
165+
@Cached ConstructListNode constructListNode) {
171166
return constructListNode.execute(frame, PythonBuiltinClassType.PList, value);
172167
}
173168

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/PythonCallNode.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@
3131
import com.oracle.graal.python.builtins.objects.function.PKeyword;
3232
import com.oracle.graal.python.builtins.objects.method.PBuiltinMethod;
3333
import com.oracle.graal.python.builtins.objects.str.StringNodes;
34+
import com.oracle.graal.python.lib.PyObjectFunctionStr;
3435
import com.oracle.graal.python.nodes.BuiltinNames;
3536
import com.oracle.graal.python.nodes.EmptyNode;
3637
import com.oracle.graal.python.nodes.ErrorMessages;
3738
import com.oracle.graal.python.nodes.PRaiseNode;
38-
import com.oracle.graal.python.nodes.SpecialAttributeNames;
3939
import com.oracle.graal.python.nodes.argument.keywords.KeywordArgumentsNode;
4040
import com.oracle.graal.python.nodes.argument.keywords.NonMappingException;
4141
import com.oracle.graal.python.nodes.argument.keywords.SameDictKeyException;
@@ -93,7 +93,7 @@ public abstract class PythonCallNode extends ExpressionNode {
9393
@Children protected final ExpressionNode[] argumentNodes;
9494
@Child private PositionalArgumentsNode positionalArguments;
9595
@Child private KeywordArgumentsNode keywordArguments;
96-
@Child private GetAttributeNode getNameAttributeNode;
96+
@Child private PyObjectFunctionStr pyObjectFunctionStr;
9797
@Child private StringNodes.CastToJavaStringCheckedNode castToStringNode;
9898

9999
protected final String calleeName;
@@ -322,23 +322,24 @@ private PKeyword[] evaluateKeywords(VirtualFrame frame, Object callable, PRaiseN
322322
CompilerDirectives.transferToInterpreterAndInvalidate();
323323
castToStringNode = insert(StringNodes.CastToJavaStringCheckedNode.create());
324324
}
325-
Object functionName = getNameAttributeNode().executeObject(frame, callable);
325+
String functionName = getFunctionName(frame, callable);
326326
String keyName = castToStringNode.execute(ex.getKey(), ErrorMessages.KEYWORDS_S_MUST_BE_STRINGS, new Object[]{functionName});
327-
throw raise.raise(PythonBuiltinClassType.TypeError, ErrorMessages.GOT_MULTIPLE_VALUES_FOR_ARG, functionName, keyName);
327+
throw raise.raise(PythonBuiltinClassType.TypeError, ErrorMessages.GOT_MULTIPLE_VALUES_FOR_KEYWORD_ARG, functionName, keyName);
328328
} catch (NonMappingException ex) {
329329
keywordsError.enter();
330-
throw raise.raise(PythonBuiltinClassType.TypeError, ErrorMessages.ARG_AFTER_MUST_BE_MAPPING, getNameAttributeNode().executeObject(frame, callable), ex.getObject());
330+
String functionName = getFunctionName(frame, callable);
331+
throw raise.raise(PythonBuiltinClassType.TypeError, ErrorMessages.ARG_AFTER_MUST_BE_MAPPING, functionName, ex.getObject());
331332
}
332333
}
333334
return result;
334335
}
335336

336-
private GetAttributeNode getNameAttributeNode() {
337-
if (getNameAttributeNode == null) {
337+
private String getFunctionName(VirtualFrame frame, Object callable) {
338+
if (pyObjectFunctionStr == null) {
338339
CompilerDirectives.transferToInterpreterAndInvalidate();
339-
getNameAttributeNode = insert(GetAttributeNode.create(SpecialAttributeNames.__NAME__));
340+
pyObjectFunctionStr = insert(PyObjectFunctionStr.create());
340341
}
341-
return getNameAttributeNode;
342+
return pyObjectFunctionStr.execute(frame, callable);
342343
}
343344

344345
@ImportStatic({PythonOptions.class})

0 commit comments

Comments
 (0)