[consider merging] Ruby binding bugfix: 'return' from block was behaving like 'break'

The problem was that Ruby uses internal exceptions to implement 'return', 'break'
and other types. These were mapped to a single tl::CancelException, rendering
the effect of 'break' and 'return' the same.

src/rba/rba/rba.cc

@@ -869,6 +869,12 @@ handle_exception (const std::string & /*where*/, rba::RubyError &ex)
   handle_exception (ex.exc (), ex.first_chance ());
 }
 
+static void
+handle_exception (rba::RubyContinueException &ex)
+{
+  rb_jump_tag (ex.state ());
+}
+
 static void
 handle_exception (const std::string &where, tl::Exception &ex)
 {
@@ -897,6 +903,8 @@ handle_exception (const std::string &where)
     handle_exception ((where), ex); \
   } catch (tl::ExitException &ex) { \
     handle_exception ((where), ex); \
+  } catch (rba::RubyContinueException &ex) { \
+    handle_exception (ex); \
   } catch (rba::RubyError &ex) { \
     handle_exception ((where), ex); \
   } catch (tl::Exception &ex) { \
@@ -1381,23 +1389,17 @@ method_adaptor (int mid, int argc, VALUE *argv, VALUE self, bool ctor)
         std::unique_ptr<gsi::IterAdaptorAbstractBase> iter ((gsi::IterAdaptorAbstractBase *) retlist.read<void *> (heap));
         if (iter.get ()) {
 
-          try {
+          gsi::SerialArgs rr (iter->serial_size ());
+          while (! iter->at_end ()) {
 
-            gsi::SerialArgs rr (iter->serial_size ());
-            while (! iter->at_end ()) {
+            rr.reset ();
+            iter->get (rr);
 
-              rr.reset ();
-              iter->get (rr);
+            VALUE value = pull_arg (meth->ret_type (), p, rr, heap);
+            rba_yield_checked (value);
 
-              VALUE value = pull_arg (meth->ret_type (), p, rr, heap);
-              rba_yield_checked (value);
+            iter->inc ();
 
-              iter->inc ();
-
-            }
-
-          } catch (tl::CancelException &) {
-            //  break encountered
           }
 
         }
@@ -2402,7 +2404,7 @@ RubyInterpreter::require (const std::string &filename_utf8)
   RUBY_END_EXEC
 
   if (error) {
-    rba_check_error ();
+    rba_check_error (error);
   }
 }
 
@@ -2423,7 +2425,7 @@ RubyInterpreter::load_file (const std::string &filename_utf8)
   RUBY_END_EXEC
 
   if (error) {
-    rba_check_error ();
+    rba_check_error (error);
   }
 }
 

src/rba/rba/rbaInternal.h

@@ -65,6 +65,26 @@ private:
   VALUE m_exc;
 };
 
+/**
+ *  @brief A class responsible for forwarding exceptions raised from "break", "return" and other flow control functions
+ */
+class RubyContinueException
+  : public tl::CancelException
+{
+public:
+  RubyContinueException (int state)
+    : tl::CancelException (), m_state (state)
+  { }
+
+  int state () const
+  {
+    return m_state;
+  }
+
+private:
+  int m_state;
+};
+
 /**
  *  @brief The proxy object that represents the C++ object on the Ruby side
  */

src/rba/rba/rbaUtils.cc

@@ -127,26 +127,23 @@ exceptions_blocked ()
 }
 
 void
-rba_check_error ()
+rba_check_error (int state)
 {
   VALUE lasterr = rb_errinfo ();
 
-  //  NOTE: this seems to be required to avoid segfaults on Ruby 2.3.1 after
-  //  a break was encountered.
-  rb_set_errinfo (Qnil);
-
-  //  Ruby employs this pseudo-exception to indicate a "break" of a loop
+  //  Ruby employs this pseudo-exception to indicate a "break" or "return" of a loop.
+  //  As this is an opaque condition, we continue Ruby execution later through a "RubyContinueException".
 #if HAVE_RUBY_VERSION_CODE < 10900
   if (lasterr == Qnil) {
-    throw tl::CancelException ();
+    throw RubyContinueException (state);
   }
 #elif HAVE_RUBY_VERSION_CODE < 20300
   if (TYPE (lasterr) == T_NODE) {
-    throw tl::CancelException ();
+    throw RubyContinueException (state);
   }
 #else
   if (TYPE (lasterr) == T_IMEMO) {
-    throw tl::CancelException ();
+    throw RubyContinueException (state);
   }
 #endif
 
@@ -405,7 +402,7 @@ rba_class_new_instance_checked (int argc, VALUE *argv, VALUE klass)
   RUBY_END_EXEC
 
   if (error) {
-    rba_check_error ();
+    rba_check_error (error);
   }
   return ret;
 }
@@ -458,7 +455,7 @@ VALUE rba_funcall2_checked (VALUE obj, ID id, int argc, VALUE *args)
   RUBY_END_EXEC
 
   if (error) {
-    rba_check_error ();
+    rba_check_error (error);
   }
   return ret;
 }
@@ -497,7 +494,7 @@ rba_f_eval_checked (int argc, VALUE *argv, VALUE self)
   RUBY_END_EXEC
 
   if (error) {
-    rba_check_error ();
+    rba_check_error (error);
   }
   return ret;
 }
@@ -513,7 +510,7 @@ rba_yield_checked (VALUE value)
   RUBY_END_EXEC
 
   if (error) {
-    rba_check_error ();
+    rba_check_error (error);
   }
 }
 

src/rba/rba/rbaUtils.h

@@ -190,7 +190,7 @@ namespace rba
 
 tl::BacktraceElement rba_split_bt_information (const char *m, size_t l);
 void rba_get_backtrace_from_array (VALUE backtrace, std::vector<tl::BacktraceElement> &bt, unsigned int skip);
-void rba_check_error ();
+void rba_check_error (int state);
 VALUE rba_string_value_f (VALUE obj);
 VALUE rba_safe_string_value (VALUE obj);
 VALUE rba_safe_obj_as_string (VALUE obj);
@@ -265,7 +265,7 @@ R rba_safe_func (R (*f) (A), A arg)
   RUBY_END_EXEC
 
   if (error) {
-    rba_check_error ();
+    rba_check_error (error);
   }
   return cp.r;
 }