Feature #18621: Fiber.yield loses the fact it was kwargs from Fiber#resume

• Status: Rejected
• Assignee: -
• Target version: -

[Eregon (Benoit Daloze)]

f = Fiber.new do
  args = Fiber.yield
  args
end
f.resume
args = f.resume(a: 1)
Hash.ruby2_keywords_hash?(args) # => false, but should be true, isn't it?

This also means if there is foo(*args) later and foo would require kwargs it would fail:

def foo(a: 1) = a

f = Fiber.new do
  args = Fiber.yield
  args
end
f.resume
args = f.resume(a: 1)
foo(*args)
# =>
# -:1:in `foo': wrong number of arguments (given 1, expected 0) (ArgumentError)
# 	from -:9:in `<main>'

cc @jeremyevans0 (Jeremy Evans) @mame (Yusuke Endoh)

[Eregon (Benoit Daloze)]

Note that kwargs are correctly passed through for Fiber.new:

Fiber.new do |*args, **kwargs|
  p kwargs # => {a: 1}
end.resume(a: 1)

[jeremyevans0 (Jeremy Evans)]

I don't think this is a bug, I think this should be expected behavior. Absent use of ruby2_keywords, in no other place in Ruby 3 should you expect the following code to work, no matter than value of args:

def foo(a: ) = a
foo(*args)

However, assuming this is considered a bug, the natural way to fix it would be to flag the hash as a ruby2_keywords hash. That's not difficult to do, but it doesn't fix the example, because ruby2_keywords processing only occurs if splatting an array, it doesn't occur if splatting a hash. So in order for it to work, the final line would need to be: foo(*[args]), or you need to change it so that multiple arguments are used:

def foo(b, a: 1) = a

f = Fiber.new do
  args = Fiber.yield
  args
end
f.resume
args = f.resume(1, a: 1)
foo(*args)

The reason for this issue is that Fiber#resume does its own argument handling (make_passing_arg in cont.c), returning the passed argument instead of an array of passed arguments if passed a single argument.

Here's a diff if we still want to make this change:

diff --git a/cont.c b/cont.c
index f9ebb4483e..4d9e2e94d0 100644
--- a/cont.c
+++ b/cont.c
@@ -29,6 +29,7 @@ extern int madvise(caddr_t, size_t, int);
 #include "gc.h"
 #include "internal.h"
 #include "internal/cont.h"
+#include "internal/hash.h"
 #include "internal/proc.h"
 #include "internal/warnings.h"
 #include "ruby/fiber/scheduler.h"
@@ -2242,6 +2243,13 @@ fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int kw_splat, rb_fi
     rb_context_t *cont = &fiber->cont;
     rb_thread_t *th = GET_THREAD();

+    if (kw_splat && argc >= 0) {
+        VALUE last_arg = argv[argc-1];
+        if (RB_TYPE_P(last_arg, T_HASH)) {
+            ((struct RHash *)last_arg)->basic.flags |= RHASH_PASS_AS_KEYWORDS;
+        }
+    }
+
     /* make sure the root_fiber object is available */
     if (th->root_fiber == NULL) root_fiber_alloc(th);

Be aware that if you go this direction, even if we remove the ruby2_keywords methods, the support for automatically treating hashes as keywords can never be removed, it must remain part of Ruby forever.

[Eregon (Benoit Daloze)]

• Tracker changed from Bug to Feature
• Status changed from Open to Rejected
• ruby -v deleted (ruby 3.2.0dev (2022-03-03T08:56:31Z master c1790f8c11) [x86_64-linux])
• Backport deleted (2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: REQUIRED, 3.1: REQUIRED)

So in order for it to work, the final line would need to be: foo(*[args])

Right, due to make_passing_arg indeed.

Be aware that if you go this direction, even if we remove the ruby2_keywords methods, the support for automatically treating hashes as keywords can never be removed, it must remain part of Ruby forever.

This is a very good point, I think this is not worth fixing in light of that.

I noticed this while implementing Ruby 3 kwargs in TruffleRuby, that basically the knowledge of "were kewargs passed" is lost in this case for Fiber#resume + Fiber.yield.
But it's also kind of natural since Fiber.yield already "packs" the arguments as a single Object or an Array, and there is no built-in object to represent all arguments currently: *args, **kwargs (and maybe also &block).

I'll reject this as I think it's not really needed in practice (one could pass a lambda doing the call with kwargs), and relying on the ruby2_keywords flag for this seems hacky.

[Eregon (Benoit Daloze)]

And thank you for this fast and detailed reply :)