vm: keep the exception stack clean; fix bare raise; implicit __context__

The frame exception stack is shared across a call chain, and the eval loop
pushed every raised exception onto it (so handlers can read it) but only
popped on handler completion. Exceptions that propagated out of a frame
uncaught — including a generator/genexpr's completion StopIteration consumed
by the FOR_ITER that resumed it — were pushed and never popped, so they
accumulated. A later bare `raise` re-raised that stale StopIteration instead
of erroring, and implicit __context__ would pick it up.

Balance the lifecycle: when an exception propagates out of a frame that has
no handler, the eval loop now pops the entry it just pushed (the caller's
eval loop re-pushes it). With the stack kept clean:
- ReRaise returns RuntimeError("No active exception to reraise") on an empty
  stack instead of asserting (bare `raise` outside a handler).
- The top-level driver uses the propagated result value rather than popping
  the exception off the (now-empty) stack.
- from_iterable's manual StopIteration cleanup is removed — it is handled by
  the eval loop now, and keeping it would wrongly pop a pre-existing
  exception when iterating during exception handling.
- RaiseVarargs re-enables implicit __context__ chaining (set the new
  exception's context to the one being handled); now reliable since the
  stack no longer holds stale state.

exception_chaining.py now covers implicit __context__, bare `raise` ->
RuntimeError, and iterating generators/comprehensions while handling an
exception.

Author: gf712

integration/tests/exception_chaining.py

@@ -1,11 +1,9 @@
-# Regression: explicit exception chaining via `raise X from Y` exposes
-# __cause__/__context__/__suppress_context__.
-#
-# NB: implicit __context__ chaining (auto-set to the exception being handled) is
-# deliberately NOT done yet — the frame exception stack isn't reliably popped
-# (internally-consumed StopIterations linger; the same bug breaks bare `raise`
-# outside a handler), so the attribute is exposed and settable but defaults to
-# None rather than being populated from that unreliable state.
+# Regression: exception chaining — __cause__ (explicit, via `raise X from Y`),
+# implicit __context__ (the exception being handled when a new one is raised),
+# and __suppress_context__. Also covers the exception-stack hygiene that makes
+# these reliable: internally-consumed StopIterations no longer linger, so a bare
+# `raise` outside a handler is a RuntimeError and __context__ isn't spuriously
+# populated.
 
 
 # `raise X from Y` sets __cause__ to the instance and suppresses context.
@@ -18,6 +16,21 @@
     assert isinstance(k.__cause__, ValueError), k.__cause__
     assert str(k.__cause__) == "inner", str(k.__cause__)
     assert k.__suppress_context__ is True, k.__suppress_context__
+    # __context__ is still set implicitly (suppress only affects display).
+    assert isinstance(k.__context__, ValueError), k.__context__
+
+
+# Implicit chaining without `from`: __context__ is the handled exception.
+try:
+    try:
+        raise ValueError("v1")
+    except ValueError:
+        raise KeyError("k1")
+except KeyError as k:
+    assert k.__cause__ is None, k.__cause__
+    assert isinstance(k.__context__, ValueError), k.__context__
+    assert str(k.__context__) == "v1", str(k.__context__)
+    assert k.__suppress_context__ is False, k.__suppress_context__
 
 
 # `raise X from None` -> cause None, still suppressed.
@@ -28,7 +41,7 @@
     assert k.__suppress_context__ is True, k.__suppress_context__
 
 
-# Plain exception: all three default cleanly.
+# Plain exception raised outside any handler: no cause, no context.
 try:
     raise ValueError("plain")
 except ValueError as e:
@@ -37,7 +50,32 @@
     assert e.__suppress_context__ is False, e.__suppress_context__
 
 
-# The chaining attributes are writable; setting __cause__ also suppresses context.
+# A bare `raise` with no active exception is a RuntimeError (not an abort, and
+# not a stale leftover exception).
+try:
+    raise
+except RuntimeError as e:
+    assert str(e) == "No active exception to reraise", str(e)
+
+
+# Iterating a generator / comprehensions while handling an exception must not
+# disturb the active exception (exception-stack hygiene).
+def gen():
+    yield 1
+    yield 2
+    yield 3
+
+
+try:
+    raise ValueError("active")
+except ValueError as e:
+    assert set(gen()) == {1, 2, 3}
+    assert [x for x in range(4)] == [0, 1, 2, 3]
+    assert {k: k * k for k in range(3)} == {0: 0, 1: 1, 2: 4}
+    assert isinstance(e, ValueError) and str(e) == "active"
+
+
+# Chaining attributes are writable; setting __cause__ also suppresses context.
 try:
     raise ValueError("x")
 except ValueError as e:
@@ -51,12 +89,4 @@
     assert e.__suppress_context__ is False
 
 
-# `from` works with a freshly-constructed cause too.
-try:
-    raise ValueError("v") from TypeError("t")
-except ValueError as e:
-    assert isinstance(e.__cause__, TypeError), e.__cause__
-    assert str(e.__cause__) == "t", str(e.__cause__)
-
-
 print("EXCEPTION_CHAINING_OK")

src/executable/bytecode/Bytecode.cpp

@@ -180,6 +180,14 @@ py::PyResult<py::Value> Bytecode::eval_loop(VirtualMachine &vm, Interpreter &int
 			ASSERT(vm.state().cleanup.size() > 0);
 			if (!vm.state().cleanup.top()) {
 				ASSERT(vm.state().cleanup.size() == 1);
+				// No handler in this frame: the exception propagates to the caller,
+				// whose eval loop re-pushes it. Pop the entry we just pushed so it
+				// does not linger on the frame-shared exception stack. Otherwise
+				// internally-consumed exceptions (e.g. a generator's completion
+				// StopIteration, swallowed by the FOR_ITER that resumed it) accumulate,
+				// and a later bare `raise` or implicit __context__ lookup observes that
+				// stale exception instead of seeing an empty stack.
+				interpreter.execution_frame()->pop_exception();
 				// when a function returns without handling the exception do not copy the value
 				// to the callers the return register
 				vm.pop_frame(false);

src/executable/bytecode/BytecodeProgram.cpp

@@ -137,19 +137,11 @@ int BytecodeProgram::execute(VirtualMachine *vm)
 	auto result = m_main_function->function()->call(*vm, interpreter);
 
 	if (result.is_err()) {
-		auto *exception = interpreter.execution_frame()->pop_exception();
-		ASSERT(exception == result.unwrap_err());
+		// The exception propagated all the way out; the eval loop already popped it
+		// off the (now-clean) exception stack as it unwound, so use the result value
+		// directly rather than popping again.
+		auto *exception = result.unwrap_err();
 		std::cout << exception->format_traceback() << std::endl;
-
-		// if (interpreter.execution_frame()->exception_info().has_value()) {
-		// 	std::cout << "During handling of the above exception, another exception occurred:\n\n";
-		// 	exception = interpreter.execution_frame()->pop_exception();
-		// 	std::cout << exception->format_traceback() << std::endl;
-		// 	if (interpreter.execution_frame()->exception_info().has_value()) {
-		// 		// how many exceptions is one meant to expect? :(
-		// 		TODO();
-		// 	}
-		// }
 	}
 
 	return result.is_ok() ? EXIT_SUCCESS : EXIT_FAILURE;

src/executable/bytecode/instructions/RaiseVarargs.cpp

@@ -39,12 +39,14 @@ PyResult<Value> RaiseVarargs::execute(VirtualMachine &vm, Interpreter &) const
 			raised->set_cause(cause.unwrap());
 			raised->set_suppress_context(true);
 		}
-		// NB: implicit __context__ chaining (set the new exception's context to
-		// the one currently being handled) is intentionally NOT done here: the
-		// frame exception stack is not reliably popped (internally-consumed
-		// StopIterations linger — the same bug that makes a bare `raise` outside
-		// a handler re-raise a stale exception), so reading it would attach a
-		// spurious context. __context__ remains exposed and settable.
+		// Implicit __context__ chaining: if an exception is currently being
+		// handled, it becomes the context of the newly-raised one (unless it is
+		// the same object). The exception stack is now kept clean, so this only
+		// fires inside a genuine handler, not from stale internal state.
+		if (auto info = vm.interpreter().execution_frame()->exception_info();
+			info.has_value() && info->exception != raised) {
+			raised->set_context(info->exception);
+		}
 		return Err(raised);
 	} else {
 		// reraise

src/executable/bytecode/instructions/ReRaise.cpp

@@ -2,13 +2,19 @@
 #include "interpreter/Interpreter.hpp"
 #include "runtime/PyFrame.hpp"
 #include "runtime/PyNone.hpp"
+#include "runtime/RuntimeError.hpp"
 
 using namespace py;
 
 
 PyResult<Value> ReRaise::execute(VirtualMachine &, Interpreter &interpreter) const
 {
-	ASSERT(interpreter.execution_frame()->exception_info().has_value());
+	// A bare `raise` with no exception currently being handled is a RuntimeError,
+	// not an abort. (Now that the exception stack is kept clean, the stack is
+	// genuinely empty here rather than holding a stale internal exception.)
+	if (!interpreter.execution_frame()->exception_info().has_value()) {
+		return Err(runtime_error("No active exception to reraise"));
+	}
 	return Err(interpreter.execution_frame()->pop_exception());
 }
 

src/runtime/utilities.hpp

@@ -21,9 +21,6 @@ PyResult<std::monostate> from_iterable(PyObject *iterable, OutputIterator result
 	static constexpr bool OutputIteratorCanError =
 		detail::has_output_iterator_error<OutputIterator>;
 
-	auto &vm = VirtualMachine::the();
-	auto exc = vm.interpreter().execution_frame()->exception_info();
-
 	auto iterator = iterable->iter();
 	if (iterator.is_err()) return Err(iterator.unwrap_err());
 
@@ -39,16 +36,9 @@ PyResult<std::monostate> from_iterable(PyObject *iterable, OutputIterator result
 
 	if (value.unwrap_err()->type() != stop_iteration()->type()) { return Err(value.unwrap_err()); }
 
-	auto current_exc = vm.interpreter().execution_frame()->exception_info();
-	if (current_exc.has_value()) {
-		auto *popped_exc = vm.interpreter().execution_frame()->pop_exception();
-		if (exc.has_value()) {
-			ASSERT(popped_exc == exc->exception);
-		} else {
-			ASSERT(!vm.interpreter().execution_frame()->exception_info().has_value());
-		}
-	}
-
+	// The iteration-terminating StopIteration does not need manual cleanup: a
+	// Python iterator's StopIteration is popped by the eval loop as it unwinds the
+	// iterator frame, and a built-in iterator never pushes one.
 	return Ok(std::monostate{});
 }