[GR-23254] Call __init__ when using raise <type>

msimacek · msimacek · commit 54aa994071ba · 2020-05-26T13:01:30.000Z
PullRequest: graalpython/969
diff --git a/graalpython/com.oracle.graal.python.benchmarks/python/micro/try-except-sized.py b/graalpython/com.oracle.graal.python.benchmarks/python/micro/try-except-sized.py
diff --git a/graalpython/com.oracle.graal.python.benchmarks/python/micro/try-except-store-sized.py b/graalpython/com.oracle.graal.python.benchmarks/python/micro/try-except-store-sized.py
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java
@@ -2924,9 +2924,14 @@ Object call() {
     @Builtin(name = "BaseException", constructsClass = PythonBuiltinClassType.PBaseException, isPublic = true, minNumOfPositionalArgs = 1, takesVarArgs = true, takesVarKeywordArgs = true)
     @GenerateNodeFactory
     public abstract static class BaseExceptionNode extends PythonBuiltinNode {
-        @Specialization
-        Object callGeneric(LazyPythonClass cls, Object[] varargs, @SuppressWarnings("unused") PKeyword[] kwargs) {
-            return factory().createBaseException(cls, factory().createTuple(varargs));
+        @Specialization(guards = "args.length == 0")
+        Object initNoArgs(LazyPythonClass cls, @SuppressWarnings("unused") Object[] args, @SuppressWarnings("unused") PKeyword[] kwargs) {
+            return factory().createBaseException(cls);
+        }
+
+        @Specialization(guards = "args.length != 0")
+        Object initArgs(LazyPythonClass cls, Object[] args, @SuppressWarnings("unused") PKeyword[] kwargs) {
+            return factory().createBaseException(cls, factory().createTuple(args));
         }
     }
 
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/BaseExceptionBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/BaseExceptionBuiltins.java
@@ -80,8 +80,13 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
     @Builtin(name = __INIT__, minNumOfPositionalArgs = 1, takesVarArgs = true)
     @GenerateNodeFactory
     public abstract static class InitNode extends PythonBuiltinNode {
-        @Specialization
-        Object init(PBaseException self, Object[] args) {
+        @Specialization(guards = "args.length == 0")
+        Object initNoArgs(@SuppressWarnings("unused") PBaseException self, @SuppressWarnings("unused") Object[] args) {
+            return PNone.NONE;
+        }
+
+        @Specialization(guards = "args.length != 0")
+        Object initArgs(PBaseException self, Object[] args) {
             self.setArgs(factory().createTuple(args));
             return PNone.NONE;
         }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/statement/RaiseNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/statement/RaiseNode.java
@@ -35,10 +35,10 @@
 import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
 import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.nodes.PRaiseNode;
+import com.oracle.graal.python.nodes.call.CallNode;
 import com.oracle.graal.python.nodes.expression.ExpressionNode;
 import com.oracle.graal.python.nodes.util.ExceptionStateNodes.GetCaughtExceptionNode;
 import com.oracle.graal.python.runtime.exception.PException;
-import com.oracle.graal.python.runtime.object.PythonObjectFactory;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.ImportStatic;
 import com.oracle.truffle.api.dsl.NodeChild;
@@ -66,16 +66,25 @@ void setCause(@SuppressWarnings("unused") VirtualFrame frame, PBaseException exc
         // raise * from <class>
         @Specialization
         void setCause(@SuppressWarnings("unused") VirtualFrame frame, PBaseException exception, LazyPythonClass causeClass,
-                        @Cached PythonObjectFactory factory,
                         @Cached BranchProfile baseCheckFailedProfile,
                         @Cached ValidExceptionNode validException,
+                        @Cached CallNode callConstructor,
                         @Cached PRaiseNode raise) {
             if (!validException.execute(frame, causeClass)) {
                 baseCheckFailedProfile.enter();
                 throw raise.raise(PythonBuiltinClassType.TypeError, "exception causes must derive from BaseException");
             }
-            PBaseException cause = factory.createBaseException(causeClass);
-            exception.setCause(cause);
+            Object cause = callConstructor.execute(causeClass);
+            if (cause instanceof PBaseException) {
+                exception.setCause((PBaseException) cause);
+            } else {
+                // msimacek: CPython currently (3.8.2) has a bug that it's not checking the type of
+                // the value returned by the constructor, you can set a non-exception as a cause
+                // this way and see the exception printer TypeError on it. I don't want to raise an
+                // exception because CPython doesn't do it and I don't want to change the cause type
+                // to Object either, because it's not meant to be that way.
+                exception.setCause(null);
+            }
         }
 
         // raise * from None
@@ -146,22 +155,34 @@ private void checkBaseClass(VirtualFrame frame, PythonAbstractClass pythonClass,
     @Specialization(guards = "isNoValue(cause)")
     void doRaise(@SuppressWarnings("unused") VirtualFrame frame, PythonAbstractClass pythonClass, @SuppressWarnings("unused") PNone cause,
                     @Cached ValidExceptionNode validException,
+                    @Cached CallNode callConstructor,
+                    @Cached BranchProfile constructorTypeErrorProfile,
                     @Cached PRaiseNode raise) {
         checkBaseClass(frame, pythonClass, validException, raise);
-        throw raise.raise(pythonClass);
+        Object newException = callConstructor.execute(frame, pythonClass);
+        if (newException instanceof PBaseException) {
+            throw raise.raise((PBaseException) newException);
+        } else {
+            constructorTypeErrorProfile.enter();
+            throw raise.raise(TypeError, "calling %s should have returned an instance of BaseException, not %p", pythonClass, newException);
+        }
     }
 
     // raise <class> from *
     @Specialization(guards = "!isNoValue(cause)")
     void doRaise(@SuppressWarnings("unused") VirtualFrame frame, PythonAbstractClass pythonClass, Object cause,
-                    @Cached PythonObjectFactory factory,
                     @Cached ValidExceptionNode validException,
                     @Cached PRaiseNode raise,
+                    @Cached CallNode callConstructor,
                     @Cached SetExceptionCauseNode setExceptionCauseNode) {
         checkBaseClass(frame, pythonClass, validException, raise);
-        PBaseException pythonException = factory.createBaseException(pythonClass);
-        setExceptionCauseNode.execute(frame, pythonException, cause);
-        throw raise.raise(pythonException);
+        Object newException = callConstructor.execute(frame, pythonClass);
+        if (newException instanceof PBaseException) {
+            setExceptionCauseNode.execute(frame, (PBaseException) newException, cause);
+            throw raise.raise((PBaseException) newException);
+        } else {
+            throw raise.raise(TypeError, "calling %s should have returned an instance of BaseException, not %p", pythonClass, newException);
+        }
     }
 
     // raise <invalid> [from *]
diff --git a/graalpython/lib-graalpython/exceptions.py b/graalpython/lib-graalpython/exceptions.py
@@ -55,30 +55,14 @@ def SystemExit__init__(self, *args):
 SystemExit.__init__ = SystemExit__init__
 del SystemExit__init__
 
-def ImportError__init__(self, msg, name=None, path=None):
-    self.message = msg
+def ImportError__init__(self, msg=None, /, *args, name=None, path=None):
+    self.msg = msg
     self.name = name
     self.path = path
 
 ImportError.__init__ = ImportError__init__
 del ImportError__init__
 
-def ModuleNotFoundError__init__(self, msg, name=None):
-    self.msg = msg
-    self.name = name
-
-ModuleNotFoundError.__init__ = ModuleNotFoundError__init__
-del ModuleNotFoundError__init__
-
-def ModuleNotFoundError__str__(self):
-    if self.name is not None:
-        return "ModuleNotFound: '" + self.name + "'. " + self.msg
-    else:
-        return "ModuleNotFound: " + self.msg
-
-ModuleNotFoundError__str__.__init__ = ModuleNotFoundError__str__
-del ModuleNotFoundError__str__
-
 # EnvironmentError is just an alias of OSError (i.e. 'EnvironmentError is OSError == True')
 EnvironmentError = OSError
 
diff --git a/mx.graalpython/mx_graalpython_bench_param.py b/mx.graalpython/mx_graalpython_bench_param.py
@@ -89,9 +89,7 @@
     'mmap-anonymous-sized': ITER_10 + ['20_000'],
     'mmap-file': ITER_10 + ['1000'],
     'generate-functions-sized': ITER_15 + ['500_000_000'],
-    'try-except-sized': ITER_10 + ['100_000_000'],
     'try-except-simple': ITER_10 + ['500_000_000'],
-    'try-except-store-sized': ITER_10 + ['100_000_000'],
     'try-except-store-simple': ITER_10 + ['500_000_000'],
     'try-except-store-two-types': ITER_10 + ['100_000_000'],
     'try-except-two-types': ITER_10 + ['100_000_000'],
diff --git a/mx.graalpython/suite.py b/mx.graalpython/suite.py
@@ -136,14 +136,6 @@
             "source": [],
         },
 
-        "perf.benchmarks": {
-            "type": "python",
-            "path": 'graalpython/com.oracle.graal.python.benchmarks',
-            "source": [
-                "python"
-            ],
-        },
-
         "util.scripts": {
             "type": "python",
             "path": 'scripts',
