Skip to content

Commit f8758a5

Browse files
msimacekmsimacek
committed
[GR-23207] Check arguments in __init__ and __new__
PullRequest: graalpython/1105
2 parents e9af51f + 713a6f4 commit f8758a5

File tree

11 files changed

+167
-39
lines changed

11 files changed

+167
-39
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_class.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
*graalpython.lib-python.3.test.test_class.ClassTests.testBadTypeReturned
22
*graalpython.lib-python.3.test.test_class.ClassTests.testBinaryOps
3+
*graalpython.lib-python.3.test.test_class.ClassTests.testConstructorErrorMessages
34
*graalpython.lib-python.3.test.test_class.ClassTests.testForExceptionsRaisedInInstanceGetattr2
45
*graalpython.lib-python.3.test.test_class.ClassTests.testGetSetAndDel
56
*graalpython.lib-python.3.test.test_class.ClassTests.testHashStuff

‎graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_generators.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
*graalpython.lib-python.3.test.test_generators.ExceptionTest.test_return_tuple
77
*graalpython.lib-python.3.test.test_generators.ExceptionTest.test_stopiteration_error
88
*graalpython.lib-python.3.test.test_generators.ExceptionTest.test_tutorial_stopiteration
9+
*graalpython.lib-python.3.test.test_generators.FinalizationTest.test_frame_resurrect
910
*graalpython.lib-python.3.test.test_generators.FinalizationTest.test_lambda_generator
11+
*graalpython.lib-python.3.test.test_generators.FinalizationTest.test_refcycle
1012
*graalpython.lib-python.3.test.test_generators.GeneratorTest.test_copy
1113
*graalpython.lib-python.3.test.test_generators.GeneratorTest.test_name
1214
*graalpython.lib-python.3.test.test_generators.GeneratorTest.test_pickle

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@
7676
import static com.oracle.graal.python.nodes.SpecialMethodNames.__EQ__;
7777
import static com.oracle.graal.python.nodes.SpecialMethodNames.__HASH__;
7878
import static com.oracle.graal.python.nodes.SpecialMethodNames.__INDEX__;
79+
import static com.oracle.graal.python.nodes.SpecialMethodNames.__INIT__;
7980
import static com.oracle.graal.python.nodes.SpecialMethodNames.__INT__;
81+
import static com.oracle.graal.python.nodes.SpecialMethodNames.__NEW__;
8082
import static com.oracle.graal.python.nodes.SpecialMethodNames.__REPR__;
8183
import static com.oracle.graal.python.nodes.SpecialMethodNames.__SETITEM__;
8284
import static com.oracle.graal.python.nodes.SpecialMethodNames.__TRUNC__;
@@ -143,6 +145,8 @@
143145
import com.oracle.graal.python.builtins.objects.memoryview.PBuffer;
144146
import com.oracle.graal.python.builtins.objects.memoryview.PMemoryView;
145147
import com.oracle.graal.python.builtins.objects.module.PythonModule;
148+
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltins;
149+
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltinsFactory;
146150
import com.oracle.graal.python.builtins.objects.object.PythonObject;
147151
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
148152
import com.oracle.graal.python.builtins.objects.range.PBigRange;
@@ -171,6 +175,7 @@
171175
import com.oracle.graal.python.nodes.SpecialMethodNames;
172176
import com.oracle.graal.python.nodes.attributes.GetAttributeNode;
173177
import com.oracle.graal.python.nodes.attributes.GetAttributeNode.GetAnyAttributeNode;
178+
import com.oracle.graal.python.nodes.attributes.LookupAttributeInMRONode;
174179
import com.oracle.graal.python.nodes.attributes.LookupInheritedAttributeNode;
175180
import com.oracle.graal.python.nodes.attributes.ReadAttributeFromObjectNode;
176181
import com.oracle.graal.python.nodes.attributes.SetAttributeNode;
@@ -229,6 +234,7 @@
229234
import com.oracle.truffle.api.nodes.UnexpectedResultException;
230235
import com.oracle.truffle.api.profiles.BranchProfile;
231236
import com.oracle.truffle.api.profiles.ConditionProfile;
237+
import com.oracle.truffle.api.profiles.ValueProfile;
232238

233239
@CoreFunctions(defineModule = BuiltinNames.BUILTINS)
234240
public final class BuiltinConstructors extends PythonBuiltins {
@@ -1669,6 +1675,10 @@ public abstract static class ObjectNode extends PythonVarargsBuiltinNode {
16691675
@Children private CExtNodes.ToSulongNode[] toSulongNodes;
16701676
@Child private CExtNodes.AsPythonObjectNode asPythonObjectNode;
16711677
@Child private SplitArgsNode splitArgsNode;
1678+
@Child private LookupAttributeInMRONode lookupInit;
1679+
@Child private LookupAttributeInMRONode lookupNew;
1680+
@CompilationFinal private ValueProfile profileInit;
1681+
@CompilationFinal private ValueProfile profileNew;
16721682

16731683
@Override
16741684
public final Object varArgExecute(VirtualFrame frame, @SuppressWarnings("unused") Object self, Object[] arguments, PKeyword[] keywords) throws VarargsBuiltinDirectInvocationNotSupported {
@@ -1680,36 +1690,28 @@ public final Object varArgExecute(VirtualFrame frame, @SuppressWarnings("unused"
16801690
}
16811691

16821692
@Specialization(guards = {"!self.needsNativeAllocation()"})
1683-
Object doManagedObject(@SuppressWarnings("unused") PythonManagedClass self, Object[] varargs, PKeyword[] kwargs) {
1684-
if (varargs.length > 0 || kwargs.length > 0) {
1685-
// TODO: tfel: this should throw an error only if init isn't overridden
1686-
}
1693+
Object doManagedObject(PythonManagedClass self, Object[] varargs, PKeyword[] kwargs) {
1694+
checkExcessArgs(self, varargs, kwargs);
16871695
return factory().createPythonObject(self);
16881696
}
16891697

16901698
@Specialization
16911699
Object doBuiltinTypeType(PythonBuiltinClassType self, Object[] varargs, PKeyword[] kwargs) {
1692-
if (varargs.length > 0 || kwargs.length > 0) {
1693-
// TODO: tfel: this should throw an error only if init isn't overridden
1694-
}
1700+
checkExcessArgs(self, varargs, kwargs);
16951701
return factory().createPythonObject(self);
16961702
}
16971703

16981704
@Specialization(guards = "self.needsNativeAllocation()")
16991705
Object doNativeObjectIndirect(PythonManagedClass self, Object[] varargs, PKeyword[] kwargs,
17001706
@Cached("create()") GetMroNode getMroNode) {
1701-
if (varargs.length > 0 || kwargs.length > 0) {
1702-
// TODO: tfel: this should throw an error only if init isn't overridden
1703-
}
1707+
checkExcessArgs(self, varargs, kwargs);
17041708
PythonNativeClass nativeBaseClass = findFirstNativeBaseClass(getMroNode.execute(self));
17051709
return callNativeGenericNewNode(nativeBaseClass, varargs, kwargs);
17061710
}
17071711

17081712
@Specialization(guards = "isNativeClass(self)")
17091713
Object doNativeObjectIndirect(Object self, Object[] varargs, PKeyword[] kwargs) {
1710-
if (varargs.length > 0 || kwargs.length > 0) {
1711-
// TODO: tfel: this should throw an error only if init isn't overridden
1712-
}
1714+
checkExcessArgs(self, varargs, kwargs);
17131715
return callNativeGenericNewNode(self, varargs, kwargs);
17141716
}
17151717

@@ -1752,6 +1754,33 @@ private Object callNativeGenericNewNode(Object self, Object[] varargs, PKeyword[
17521754
callCapiFunction.call(FUN_PY_OBJECT_NEW, toSulongNodes[0].execute(self), toSulongNodes[1].execute(self), toSulongNodes[2].execute(targs),
17531755
toSulongNodes[3].execute(dkwargs)));
17541756
}
1757+
1758+
private void checkExcessArgs(Object type, Object[] varargs, PKeyword[] kwargs) {
1759+
if (varargs.length != 0 || kwargs.length != 0) {
1760+
if (lookupNew == null) {
1761+
CompilerDirectives.transferToInterpreterAndInvalidate();
1762+
lookupNew = insert(LookupAttributeInMRONode.create(__NEW__));
1763+
}
1764+
if (lookupInit == null) {
1765+
CompilerDirectives.transferToInterpreterAndInvalidate();
1766+
lookupInit = insert(LookupAttributeInMRONode.create(__INIT__));
1767+
}
1768+
if (profileNew == null) {
1769+
CompilerDirectives.transferToInterpreterAndInvalidate();
1770+
profileNew = ValueProfile.createIdentityProfile();
1771+
}
1772+
if (profileInit == null) {
1773+
CompilerDirectives.transferToInterpreterAndInvalidate();
1774+
profileInit = ValueProfile.createIdentityProfile();
1775+
}
1776+
if (ObjectBuiltins.InitNode.overridesBuiltinMethod(type, profileNew, lookupNew, BuiltinConstructorsFactory.ObjectNodeFactory.class)) {
1777+
throw raise(TypeError, ErrorMessages.NEW_TAKES_ONE_ARG);
1778+
}
1779+
if (!ObjectBuiltins.InitNode.overridesBuiltinMethod(type, profileInit, lookupInit, ObjectBuiltinsFactory.InitNodeFactory.class)) {
1780+
throw raise(TypeError, ErrorMessages.NEW_TAKES_NO_ARGS, type);
1781+
}
1782+
}
1783+
}
17551784
}
17561785

17571786
// range(stop)

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/object/ObjectBuiltins.java

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import com.oracle.graal.python.builtins.CoreFunctions;
5454
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
5555
import com.oracle.graal.python.builtins.PythonBuiltins;
56+
import com.oracle.graal.python.builtins.modules.BuiltinConstructorsFactory;
5657
import com.oracle.graal.python.builtins.objects.PNone;
5758
import com.oracle.graal.python.builtins.objects.PNotImplemented;
5859
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
@@ -91,6 +92,7 @@
9192
import com.oracle.graal.python.nodes.function.builtins.PythonVarargsBuiltinNode;
9293
import com.oracle.graal.python.nodes.object.GetClassNode;
9394
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
95+
import com.oracle.graal.python.nodes.util.SplitArgsNode;
9496
import com.oracle.truffle.api.CompilerDirectives;
9597
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
9698
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -107,6 +109,7 @@
107109
import com.oracle.truffle.api.nodes.UnexpectedResultException;
108110
import com.oracle.truffle.api.profiles.BranchProfile;
109111
import com.oracle.truffle.api.profiles.ConditionProfile;
112+
import com.oracle.truffle.api.profiles.ValueProfile;
110113

111114
@CoreFunctions(extendClasses = PythonBuiltinClassType.PythonObject)
112115
public class ObjectBuiltins extends PythonBuiltins {
@@ -183,18 +186,53 @@ private CheckCompatibleForAssigmentNode getCheckCompatibleForAssigmentNode() {
183186
@Builtin(name = __INIT__, takesVarArgs = true, minNumOfPositionalArgs = 1, takesVarKeywordArgs = true)
184187
@GenerateNodeFactory
185188
public abstract static class InitNode extends PythonVarargsBuiltinNode {
189+
@Child private SplitArgsNode splitArgsNode;
190+
186191
@Override
187192
public final Object varArgExecute(VirtualFrame frame, @SuppressWarnings("unused") Object self, Object[] arguments, PKeyword[] keywords) throws VarargsBuiltinDirectInvocationNotSupported {
193+
if (splitArgsNode == null) {
194+
CompilerDirectives.transferToInterpreterAndInvalidate();
195+
splitArgsNode = insert(SplitArgsNode.create());
196+
}
197+
return execute(frame, arguments[0], splitArgsNode.execute(arguments), keywords);
198+
}
199+
200+
@Specialization(guards = {"arguments.length == 0", "keywords.length == 0"})
201+
@SuppressWarnings("unused")
202+
public PNone initNoArgs(Object self, Object[] arguments, PKeyword[] keywords) {
188203
return PNone.NONE;
189204
}
190205

191-
@Specialization
206+
@Specialization(replaces = "initNoArgs", limit = "3")
192207
@SuppressWarnings("unused")
193-
public PNone init(Object self, Object[] arguments, PKeyword[] keywords) {
194-
// TODO: tfel: throw an error if we get additional arguments and the __new__
195-
// method was the same as object.__new__
208+
public PNone init(Object self, Object[] arguments, PKeyword[] keywords,
209+
@CachedLibrary("self") PythonObjectLibrary lib,
210+
@Cached("create(__INIT__)") LookupAttributeInMRONode lookupInit,
211+
@Cached("createIdentityProfile()") ValueProfile profileInit,
212+
@Cached("create(__NEW__)") LookupAttributeInMRONode lookupNew,
213+
@Cached("createIdentityProfile()") ValueProfile profileNew) {
214+
if (arguments.length != 0 || keywords.length != 0) {
215+
Object type = lib.getLazyPythonClass(self);
216+
if (overridesBuiltinMethod(type, profileInit, lookupInit, ObjectBuiltinsFactory.InitNodeFactory.class)) {
217+
throw raise(TypeError, ErrorMessages.INIT_TAKES_ONE_ARG_OBJECT);
218+
}
219+
220+
if (!overridesBuiltinMethod(type, profileInit, lookupNew, BuiltinConstructorsFactory.ObjectNodeFactory.class)) {
221+
throw raise(TypeError, ErrorMessages.INIT_TAKES_ONE_ARG, type);
222+
}
223+
}
196224
return PNone.NONE;
197225
}
226+
227+
public static <T extends NodeFactory<? extends PythonBuiltinBaseNode>> boolean overridesBuiltinMethod(Object type, ValueProfile profile, LookupAttributeInMRONode lookup,
228+
Class<T> builtinNodeFactoryClass) {
229+
Object method = profile.profile(lookup.execute(type));
230+
if (method instanceof PBuiltinFunction) {
231+
NodeFactory<? extends PythonBuiltinBaseNode> factory = ((PBuiltinFunction) method).getBuiltinNodeFactory();
232+
return !builtinNodeFactoryClass.isInstance(factory);
233+
}
234+
return true;
235+
}
198236
}
199237

200238
@Builtin(name = __HASH__, minNumOfPositionalArgs = 1)

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/referencetype/ReferenceTypeBuiltins.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import static com.oracle.graal.python.nodes.SpecialMethodNames.__CALL__;
4545
import static com.oracle.graal.python.nodes.SpecialMethodNames.__EQ__;
4646
import static com.oracle.graal.python.nodes.SpecialMethodNames.__HASH__;
47+
import static com.oracle.graal.python.nodes.SpecialMethodNames.__INIT__;
4748
import static com.oracle.graal.python.nodes.SpecialMethodNames.__NE__;
4849
import static com.oracle.graal.python.nodes.SpecialMethodNames.__REPR__;
4950

@@ -60,6 +61,7 @@
6061
import com.oracle.graal.python.nodes.expression.BinaryComparisonNode;
6162
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
6263
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
64+
import com.oracle.graal.python.nodes.function.builtins.PythonTernaryBuiltinNode;
6365
import com.oracle.graal.python.nodes.function.builtins.PythonUnaryBuiltinNode;
6466
import com.oracle.graal.python.runtime.exception.PythonErrorType;
6567
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -79,6 +81,16 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
7981
return ReferenceTypeBuiltinsFactory.getFactories();
8082
}
8183

84+
@Builtin(name = __INIT__, minNumOfPositionalArgs = 2, maxNumOfPositionalArgs = 3)
85+
@GenerateNodeFactory
86+
public abstract static class InitNode extends PythonTernaryBuiltinNode {
87+
@Specialization
88+
@SuppressWarnings("unused")
89+
Object init(Object self, Object obj, Object callback) {
90+
return PNone.NONE;
91+
}
92+
}
93+
8294
// ref.__callback__
8395
@Builtin(name = __CALLBACK__, minNumOfPositionalArgs = 1)
8496
@GenerateNodeFactory

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/socket/SocketBuiltins.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
*/
4141
package com.oracle.graal.python.builtins.objects.socket;
4242

43+
import static com.oracle.graal.python.nodes.SpecialMethodNames.__INIT__;
44+
4345
import java.io.IOException;
4446
import java.net.InetAddress;
4547
import java.net.InetSocketAddress;
@@ -96,6 +98,16 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
9698
return SocketBuiltinsFactory.getFactories();
9799
}
98100

101+
@Builtin(name = __INIT__, minNumOfPositionalArgs = 1, maxNumOfPositionalArgs = 5)
102+
@GenerateNodeFactory
103+
public abstract static class InitNode extends PythonBuiltinNode {
104+
@Specialization
105+
@SuppressWarnings("unused")
106+
Object init(Object self, Object family, Object type, Object proto, Object fileno) {
107+
return PNone.NONE;
108+
}
109+
}
110+
99111
// accept()
100112
@Builtin(name = "_accept", minNumOfPositionalArgs = 1)
101113
@GenerateNodeFactory

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeBuiltins.java

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@
110110
import com.oracle.graal.python.nodes.function.builtins.PythonVarargsBuiltinNode;
111111
import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
112112
import com.oracle.graal.python.nodes.truffle.PythonTypes;
113+
import com.oracle.graal.python.nodes.util.SplitArgsNode;
113114
import com.oracle.graal.python.runtime.exception.PException;
114115
import com.oracle.graal.python.runtime.exception.PythonErrorType;
115116
import com.oracle.truffle.api.CompilerAsserts;
@@ -200,6 +201,29 @@ Object doit(Object object,
200201
}
201202
}
202203

204+
@Builtin(name = __INIT__, takesVarArgs = true, minNumOfPositionalArgs = 1, takesVarKeywordArgs = true)
205+
@GenerateNodeFactory
206+
public abstract static class InitNode extends PythonVarargsBuiltinNode {
207+
@Child private SplitArgsNode splitArgsNode;
208+
209+
@Override
210+
public final Object varArgExecute(VirtualFrame frame, @SuppressWarnings("unused") Object self, Object[] arguments, PKeyword[] keywords) throws VarargsBuiltinDirectInvocationNotSupported {
211+
if (splitArgsNode == null) {
212+
CompilerDirectives.transferToInterpreterAndInvalidate();
213+
splitArgsNode = insert(SplitArgsNode.create());
214+
}
215+
return execute(frame, arguments[0], splitArgsNode.execute(arguments), keywords);
216+
}
217+
218+
@Specialization
219+
Object init(@SuppressWarnings("unused") Object self, Object[] arguments, @SuppressWarnings("unused") PKeyword[] kwds) {
220+
if (arguments.length != 1 && arguments.length != 3) {
221+
throw raise(TypeError, ErrorMessages.TAKES_D_OR_D_ARGS, "type.__init__()", 1, 3);
222+
}
223+
return PNone.NONE;
224+
}
225+
}
226+
203227
@Builtin(name = __CALL__, minNumOfPositionalArgs = 1, takesVarArgs = true, takesVarKeywordArgs = true)
204228
@GenerateNodeFactory
205229
@ReportPolymorphism
@@ -358,25 +382,19 @@ private Object op(VirtualFrame frame, Object self, Object[] arguments, PKeyword[
358382
}
359383
Object newInstanceKlass = lib.getLazyPythonClass(newInstance);
360384
if (isSubType(newInstanceKlass, self)) {
361-
if (arguments.length == 2 && isClassClassProfile.profileClass(self, PythonBuiltinClassType.PythonClass)) {
362-
// do not call init if we are creating a new instance of type and we are
363-
// passing keywords or more than one argument see:
364-
// https://github.com/python/cpython/blob/2102c789035ccacbac4362589402ac68baa2cd29/Objects/typeobject.c#L3538
365-
} else {
366-
Object initMethod = lookupInit.execute(frame, newInstanceKlass, newInstance);
367-
if (initMethod != PNone.NO_VALUE) {
368-
Object[] initArgs;
369-
if (doCreateArgs) {
370-
initArgs = PositionalArgumentsNode.prependArgument(newInstance, arguments);
371-
} else {
372-
// XXX: (tfel) is this valid? I think it should be fine...
373-
arguments[0] = newInstance;
374-
initArgs = arguments;
375-
}
376-
Object initResult = dispatchInit.execute(frame, initMethod, initArgs, keywords);
377-
if (initResult != PNone.NONE && initResult != PNone.NO_VALUE) {
378-
throw raise(TypeError, ErrorMessages.SHOULD_RETURN_NONE, "__init__()");
379-
}
385+
Object initMethod = lookupInit.execute(frame, newInstanceKlass, newInstance);
386+
if (initMethod != PNone.NO_VALUE) {
387+
Object[] initArgs;
388+
if (doCreateArgs) {
389+
initArgs = PositionalArgumentsNode.prependArgument(newInstance, arguments);
390+
} else {
391+
// XXX: (tfel) is this valid? I think it should be fine...
392+
arguments[0] = newInstance;
393+
initArgs = arguments;
394+
}
395+
Object initResult = dispatchInit.execute(frame, initMethod, initArgs, keywords);
396+
if (initResult != PNone.NONE && initResult != PNone.NO_VALUE) {
397+
throw raise(TypeError, ErrorMessages.SHOULD_RETURN_NONE, "__init__()");
380398
}
381399
}
382400
}

0 commit comments

Comments
 (0)