Fix exception state handling in Bytecode DSL generators

Author: steve-s

graalpython/com.oracle.graal.python.test/src/tests/test_exception.py

@@ -196,7 +196,6 @@ def test_raise_none(self):
         except TypeError:
             pass
 
-    @unittest.skipIf(os.environ.get('BYTECODE_DSL_INTERPRETER'), "TODO: bug in comment above")
     def test_generator(self):
         def gen():
             try:
@@ -208,7 +207,6 @@ def gen():
         self.assertEqual(next(g), 1)
         self.assertRaises(ZeroDivisionError, lambda: next(g))
 
-    @unittest.skipIf(os.environ.get('BYTECODE_DSL_INTERPRETER'), "TODO: bug in comment above")
     def test_generator_nested(self):
         def gen():
             try:
@@ -448,7 +446,6 @@ def foo():
         self.assertEqual(e.__context__.__context__.args[0], "first")
         self.assertIsNone(e.__context__.__context__.__context__)
 
-    @unittest.skipIf(os.environ.get('BYTECODE_DSL_INTERPRETER'), "TODO: bug in comment above")
     def test_implicit_chaining_generator(self):
         def gen():
             try:
@@ -488,7 +485,6 @@ def gen():
         self.assertEqual(e.__context__.__context__.args[0], "first")
         self.assertIsNone(e.__context__.__context__.__context__)
 
-    @unittest.skipIf(os.environ.get('BYTECODE_DSL_INTERPRETER'), "TODO: bug in comment above")
     def test_implicit_chaining_generator_finally(self):
         def gen():
             try:

graalpython/com.oracle.graal.python.test/src/tests/test_generators.py

@@ -11,7 +11,6 @@ class ExceptionTest(unittest.TestCase):
     # Tests for the issue #23353: check that the currently handled exception
     # is correctly saved/restored in PyEval_EvalFrameEx().
 
-    @unittest.skipIf(os.environ.get('BYTECODE_DSL_INTERPRETER'), "TODO: bug in comment above")
     def test_except_throw(self):
 
         def store_raise_exc_generator():
@@ -49,7 +48,6 @@ def store_raise_exc_generator():
 
         self.assertEqual(sys.exc_info(), (None, None, None))
 
-    @unittest.skipIf(os.environ.get('BYTECODE_DSL_INTERPRETER'), "TODO: bug in comment above")
     def test_except_next(self):
         def gen():
             self.assertEqual(sys.exc_info()[0], ValueError)
@@ -107,14 +105,14 @@ def gen():
     #         yield
     #         self.assertIsNone(sys.exc_info()[0])
     #         yield "done"
- 
+
     #     g = gen()
     #     next(g)
     #     try:
     #         raise ValueError
     #     except Exception as exc:
     #         g.throw(exc)
- 
+
     #     self.assertEqual(next(g), "done")
     #     self.assertEqual(sys.exc_info(), (None, None, None))
 
@@ -129,7 +127,7 @@ def gen(log):
             yield
             log.append(3)
             return
-        
+
         log = []
         g = gen(log)
         next(g)
@@ -152,7 +150,7 @@ def gen(log):
             yield
             log.append(3)
             return
-        
+
         log = []
         g = gen(log)
         next(g)
@@ -175,7 +173,7 @@ def gen(log):
             yield
             log.append(3)
             return
-        
+
         log = []
         g = gen(log)
         next(g)
@@ -191,7 +189,7 @@ def test_gen_from_except(self):
         def gen():
             self.assertEqual(sys.exc_info()[0], None)
             yield
-            
+
             try:
                 raise TypeError
             except TypeError:
@@ -201,7 +199,7 @@ def gen():
             yield
             self.assertIsNone(sys.exc_info()[0])
             yield "done"
-            
+
         try:
             raise ValueError
         except ValueError:
@@ -352,3 +350,130 @@ def illegal_state_expected_cell_got_list():
         ]
 
     assert len(illegal_state_expected_cell_got_list()) == 2
+
+def test_generator_exceptions_finally():
+    def get_exc_state():
+        assert sys.exc_info()[1] == sys.exception()
+        return sys.exception()
+
+    def generator():
+        yield get_exc_state() # 1
+        try:
+            yield get_exc_state() # 2
+            3 / 0
+        except:
+            yield get_exc_state() # 3
+            yield get_exc_state() # 4
+        finally:
+            yield get_exc_state() # 5
+            try:
+                raise NameError()
+            except:
+                yield get_exc_state() # 6
+                try:
+                    raise KeyError()
+                except:
+                    yield get_exc_state() # 7
+                yield get_exc_state() # 8
+            yield get_exc_state() # 9
+        yield get_exc_state() # 10
+
+    def run_test(check_caller_ex):
+        g = generator()
+        assert check_caller_ex(g.send(None)) # 1
+        assert check_caller_ex(g.send(None)) # 2
+        assert type(g.send(None)) == ZeroDivisionError # 3
+        assert type(g.send(None)) == ZeroDivisionError # 4
+        assert check_caller_ex(g.send(None)) # 5
+        assert type(g.send(None)) == NameError # 6
+        assert type(g.send(None)) == KeyError # 7
+        assert type(g.send(None)) == NameError # 8
+        assert check_caller_ex(g.send(None)) # 9
+        assert check_caller_ex(g.send(None)) # 10
+
+    run_test(lambda x: x is None)
+    try:
+        raise NotImplementedError()
+    except:
+        run_test(lambda x: type(x) == NotImplementedError)
+
+
+def test_generator_exceptions_complex():
+    def get_exc_state():
+        assert sys.exc_info()[1] == sys.exception()
+        return sys.exception()
+
+    def generator():
+        yield get_exc_state() # 1
+        try:
+            yield get_exc_state() # 2
+            3 / 0
+        except:
+            yield get_exc_state() # 3
+            yield get_exc_state() # 4
+        yield get_exc_state() # 5
+        yield get_exc_state() # 6
+        yield get_exc_state() # 7
+        try:
+            yield get_exc_state() # 8
+            raise KeyError()
+        except:
+            yield get_exc_state() # 9
+            yield get_exc_state() # 10
+        yield get_exc_state() # 11
+        try:
+            raise NameError()
+        except:
+            yield get_exc_state() # 12
+            try:
+                raise NotImplementedError()
+            except:
+                yield get_exc_state() # 13
+            yield get_exc_state() # 14
+        yield get_exc_state() # 15
+        yield get_exc_state() # 16
+
+    g = generator()
+    try:
+        raise AttributeError()
+    except:
+        assert type(g.send(None)) == AttributeError # 1
+        assert type(g.send(None)) == AttributeError # 2
+        assert type(g.send(None)) == ZeroDivisionError # 3
+        assert type(g.send(None)) == ZeroDivisionError # 4
+        assert type(g.send(None)) == AttributeError # 5
+    assert g.send(None) is None # 6
+    try:
+        raise IndexError()
+    except:
+        assert type(g.send(None)) == IndexError # 7
+        assert type(g.send(None)) == IndexError # 8
+        assert type(g.send(None)) == KeyError # 9
+        assert type(g.send(None)) == KeyError # 10
+        assert type(g.send(None)) == IndexError # 11
+    try:
+        raise TypeError()
+    except:
+        assert type(g.send(None)) == NameError # 12
+        assert type(g.send(None)) == NotImplementedError # 13
+        assert type(g.send(None)) == NameError # 14
+        assert type(g.send(None)) == TypeError # 15
+    assert g.send(None) is None # 16
+
+    g = generator()
+    assert g.send(None) is None # 1
+    assert g.send(None) is None # 2
+    assert type(g.send(None)) == ZeroDivisionError # 3
+    assert type(g.send(None)) == ZeroDivisionError # 4
+    assert g.send(None) is None # 5
+    assert g.send(None) is None # 6
+    assert g.send(None) is None # 7
+    assert g.send(None) is None # 8
+    assert type(g.send(None)) == KeyError # 9
+    assert type(g.send(None)) == KeyError # 10
+    assert g.send(None) is None # 11
+    assert type(g.send(None)) == NameError # 12
+    assert type(g.send(None)) == NotImplementedError # 13
+    assert type(g.send(None)) == NameError # 14
+    assert g.send(None) is None # 15
+    assert g.send(None) is None # 16

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

@@ -6,14 +6,26 @@ DocTestCase.test.test_generators.__test__.refleaks @ linux-x86_64
 DocTestCase.test.test_generators.__test__.syntax @ linux-x86_64
 DocTestCase.test.test_generators.__test__.tut @ linux-x86_64
 DocTestCase.test.test_generators.__test__.weakref @ linux-x86_64
+test.test_generators.ExceptionTest.test_except_gen_except @ linux-x86_64
+test.test_generators.ExceptionTest.test_except_next @ linux-x86_64
+test.test_generators.ExceptionTest.test_except_throw @ linux-x86_64
 test.test_generators.ExceptionTest.test_except_throw_bad_exception @ linux-x86_64
+test.test_generators.ExceptionTest.test_except_throw_exception_context @ linux-x86_64
+test.test_generators.ExceptionTest.test_gen_3_arg_deprecation_warning @ linux-x86_64
+test.test_generators.ExceptionTest.test_nested_gen_except_loop @ linux-x86_64
 test.test_generators.ExceptionTest.test_return_stopiteration @ linux-x86_64
 test.test_generators.ExceptionTest.test_return_tuple @ linux-x86_64
 test.test_generators.ExceptionTest.test_stopiteration_error @ linux-x86_64
 test.test_generators.ExceptionTest.test_tutorial_stopiteration @ linux-x86_64
 test.test_generators.GeneratorStackTraceTest.test_send_with_yield_from @ linux-x86_64
 test.test_generators.GeneratorStackTraceTest.test_throw_with_yield_from @ linux-x86_64
 test.test_generators.GeneratorTest.test_copy @ linux-x86_64
+test.test_generators.GeneratorTest.test_issue103488 @ linux-x86_64
+test.test_generators.GeneratorTest.test_name @ linux-x86_64
 test.test_generators.GeneratorTest.test_pickle @ linux-x86_64
 test.test_generators.GeneratorTest.test_send_non_none_to_new_gen @ linux-x86_64
-test.test_generators.GeneratorThrowTest.test_throw_after_none_exc_type @ linux-x86_64
+test.test_generators.GeneratorThrowTest.test_exception_context_with_yield @ linux-x86_64
+test.test_generators.GeneratorThrowTest.test_exception_context_with_yield_from @ linux-x86_64
+test.test_generators.GeneratorThrowTest.test_exception_context_with_yield_from_with_context_cycle @ linux-x86_64
+test.test_generators.GeneratorThrowTest.test_exception_context_with_yield_inside_generator @ linux-x86_64
+test.test_generators.GeneratorThrowTest.test_throw_after_none_exc_type @ linux-x86_64

graalpython/com.oracle.graal.python.test/src/tests/unittest_tags_bytecode_dsl/test_zipimport_support.txt

@@ -1,3 +1,4 @@
+savetest.test_zipimport_support.ZipSupportTests.test_doctest_issue4197 @ linux-x86_64
 test.test_zipimport_support.ZipSupportTests.test_doctest_main_issue4197 @ linux-x86_64
 test.test_zipimport_support.ZipSupportTests.test_inspect_getsource_issue4223 @ linux-x86_64
 test.test_zipimport_support.ZipSupportTests.test_pdb_issue4201 @ linux-x86_64

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/generator/CommonGeneratorBuiltins.java

@@ -165,31 +165,12 @@ static Object cachedBytecodeDSL(VirtualFrame frame, Node inliningTarget, PGenera
             try {
                 ContinuationResult continuation = self.getContinuation();
                 Object[] arguments;
-                // TODO: GR-62196, Bytecode DSL does not have the same shape of arguments array for
-                // continuation calls:
-
-                // 1) in the manual interpreter, we always pass an array of the same length (with
-                // slots defined in PArguments), this argument array is used for callee context
-                // enter/exit in PBytecodeGeneratorRootNode as opposed to the original arguments
-                // array taken from the PGenerator object. Moreover, this array is a copy of
-                // PGenerator arguments, and the comment above prepareArguments seems to indicate
-                // that we indeed need a fresh copy, because we do not want to share the state
-                // stored in the arguments between invocations
-
-                // 2) Bytecode DSL doesn't do callee context enter/exit for individual calls,
-                // but for the whole coroutine
-
-                // 3) when walking the stack, e.g., in MaterializeFrameNode, we must take care of
-                // this additional arguments shape and unwrap the materialized frame from the
-                // continuation frame to access its arguments array that will have the desired
-                // "PArguments shape", however this will be a shared arguments array, so it is a
-                // question if this unwrapping would be correct, see 1).
-
                 if (firstCall) {
                     // First invocation: call the regular root node.
                     arguments = self.getCallArguments(sendValue);
                 } else {
                     // Subsequent invocations: call a continuation root node.
+                    self.prepareResume();
                     arguments = new Object[]{continuation.getFrame(), sendValue};
                 }
                 generatorResult = invoke.execute(frame, inliningTarget, callNode, arguments);
@@ -248,6 +229,7 @@ static Object genericBytecodeDSL(VirtualFrame frame, Node inliningTarget, PGener
                     arguments = self.getCallArguments(sendValue);
                 } else {
                     // Subsequent invocations: call a continuation root node.
+                    self.prepareResume();
                     arguments = new Object[]{continuation.getFrame(), sendValue};
                 }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/generator/PGenerator.java

@@ -188,6 +188,7 @@ protected PGenerator(PythonLanguage lang, PFunction function, MaterializedFrame
 
     public Object[] getCallArguments(Object sendValue) {
         if (PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER) {
+            prepareResume();
             return getBytecodeDSLState().arguments;
         } else {
             Object[] arguments = PArguments.create(2);
@@ -199,6 +200,11 @@ public Object[] getCallArguments(Object sendValue) {
         }
     }
 
+    public void prepareResume() {
+        assert PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER; // not needed for manual interpreter
+        PArguments.setException(getBytecodeDSLState().arguments, null);
+    }
+
     public static boolean isGeneratorFrame(Frame frame) {
         if (PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER) {
             return false;