ruby-changes:72307
From: Samuel <ko1@a...>
Date: Sat, 25 Jun 2022 16:22:10 +0900 (JST)
Subject: [ruby-changes:72307] f9c8d80883 (master): [ruby/io-wait] Don't add `IO#wait*` methods when `RUBY_IO_WAIT_METHODS` is defined by Ruby. (https://github.com/ruby/io-wait/pull/19)
https://git.ruby-lang.org/ruby.git/commit/?id=f9c8d80883 From f9c8d8088378981385f6acd18d2fe9fedcd7ab85 Mon Sep 17 00:00:00 2001 From: Samuel Williams <samuel.williams@o...> Date: Sat, 25 Jun 2022 19:21:37 +1200 Subject: [ruby/io-wait] Don't add `IO#wait*` methods when `RUBY_IO_WAIT_METHODS` is defined by Ruby. (https://github.com/ruby/io-wait/pull/19) * Fix return value compatibility with Ruby 2.x. * Don't add `IO#wait*` methods in Ruby 3.2+. https://github.com/ruby/io-wait/commit/54c504d089 --- ext/io/wait/wait.c | 116 ++++++++++++++++++++++++++++--------------- test/io/wait/test_io_wait.rb | 28 +++++++++++ 2 files changed, 103 insertions(+), 41 deletions(-) diff --git a/ext/io/wait/wait.c b/ext/io/wait/wait.c index 7fb0a4a1e6..1e3b43c919 100644 --- a/ext/io/wait/wait.c +++ b/ext/io/wait/wait.c @@ -95,21 +95,24 @@ io_nread(VALUE io) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L95 #ifdef HAVE_RB_IO_WAIT static VALUE -io_wait_event(VALUE io, int event, VALUE timeout) +io_wait_event(VALUE io, int event, VALUE timeout, int return_io) { VALUE result = rb_io_wait(io, RB_INT2NUM(event), timeout); if (!RB_TEST(result)) { - return Qnil; + return Qnil; } int mask = RB_NUM2INT(result); if (mask & event) { - return io; + if (return_io) + return io; + else + return result; } else { - return Qfalse; + return Qfalse; } } #endif @@ -137,15 +140,15 @@ io_ready_p(VALUE io) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L140 if (rb_io_read_pending(fptr)) return Qtrue; #ifndef HAVE_RB_IO_WAIT - if (wait_for_single_fd(fptr, RB_WAITFD_IN, &tv)) - return Qtrue; + return wait_for_single_fd(fptr, RB_WAITFD_IN, &tv) ? Qtrue : Qfalse; #else - if (RTEST(io_wait_event(io, RUBY_IO_READABLE, RB_INT2NUM(0)))) - return Qtrue; + return io_wait_event(io, RUBY_IO_READABLE, RB_INT2NUM(0), 1); #endif - return Qfalse; } +// Ruby 3.2+ can define these methods. This macro indicates that case. +#ifndef RUBY_IO_WAIT_METHODS + /* * call-seq: * io.wait_readable -> truthy or falsy @@ -184,7 +187,7 @@ io_wait_readable(int argc, VALUE *argv, VALUE io) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L187 rb_check_arity(argc, 0, 1); VALUE timeout = (argc == 1 ? argv[0] : Qnil); - return io_wait_event(io, RUBY_IO_READABLE, timeout); + return io_wait_event(io, RUBY_IO_READABLE, timeout, 1); #endif } @@ -220,7 +223,7 @@ io_wait_writable(int argc, VALUE *argv, VALUE io) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L223 rb_check_arity(argc, 0, 1); VALUE timeout = (argc == 1 ? argv[0] : Qnil); - return io_wait_event(io, RUBY_IO_WRITABLE, timeout); + return io_wait_event(io, RUBY_IO_WRITABLE, timeout, 1); #endif } @@ -231,7 +234,8 @@ io_wait_writable(int argc, VALUE *argv, VALUE io) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L234 * io.wait_priority(timeout) -> truthy or falsy * * Waits until IO is priority and returns a truthy value or a falsy - * value when times out. + * value when times out. Priority data is sent and received using + * the Socket::MSG_OOB flag and is typically limited to streams. * * You must require 'io/wait' to use this method. */ @@ -248,7 +252,7 @@ io_wait_priority(int argc, VALUE *argv, VALUE io) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L252 rb_check_arity(argc, 0, 1); VALUE timeout = argc == 1 ? argv[0] : Qnil; - return io_wait_event(io, RUBY_IO_PRIORITY, timeout); + return io_wait_event(io, RUBY_IO_PRIORITY, timeout, 1); } #endif @@ -286,10 +290,22 @@ wait_mode_sym(VALUE mode) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L290 return 0; } +#ifdef HAVE_RB_IO_WAIT +static inline rb_io_event_t +io_event_from_value(VALUE value) +{ + int events = RB_NUM2INT(value); + + if (events <= 0) rb_raise(rb_eArgError, "Events must be positive integer!"); + + return events; +} +#endif + /* * call-seq: - * io.wait(events, timeout) -> truthy or falsy - * io.wait(timeout = nil, mode = :read) -> truthy or falsy. + * io.wait(events, timeout) -> event mask, false or nil + * io.wait(timeout = nil, mode = :read) -> self, true, or false * * Waits until the IO becomes ready for the specified events and returns the * subset of events that become ready, or a falsy value when times out. @@ -335,43 +351,59 @@ io_wait(int argc, VALUE *argv, VALUE io) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L351 #else VALUE timeout = Qundef; rb_io_event_t events = 0; + int return_io = 0; + // The documented signature for this method is actually incorrect. + // A single timeout is allowed in any position, and multiple symbols can be given. + // Whether this is intentional or not, I don't know, and as such I consider this to + // be a legacy/slow path. if (argc != 2 || (RB_SYMBOL_P(argv[0]) || RB_SYMBOL_P(argv[1]))) { - for (int i = 0; i < argc; i += 1) { - if (RB_SYMBOL_P(argv[i])) { - events |= wait_mode_sym(argv[i]); - } - else if (timeout == Qundef) { - rb_time_interval(timeout = argv[i]); - } - else { - rb_raise(rb_eArgError, "timeout given more than once"); - } - } - if (timeout == Qundef) timeout = Qnil; + // We'd prefer to return the actual mask, but this form would return the io itself: + return_io = 1; + + // Slow/messy path: + for (int i = 0; i < argc; i += 1) { + if (RB_SYMBOL_P(argv[i])) { + events |= wait_mode_sym(argv[i]); + } + else if (timeout == Qundef) { + rb_time_interval(timeout = argv[i]); + } + else { + rb_raise(rb_eArgError, "timeout given more than once"); + } + } + + if (timeout == Qundef) timeout = Qnil; + + if (events == 0) { + events = RUBY_IO_READABLE; + } } - else /* argc == 2 */ { - events = RB_NUM2UINT(argv[0]); - timeout = argv[1]; - } - - if (events == 0) { - events = RUBY_IO_READABLE; + else /* argc == 2 and neither are symbols */ { + // This is the fast path: + events = io_event_from_value(argv[0]); + timeout = argv[1]; } if (events & RUBY_IO_READABLE) { - rb_io_t *fptr = NULL; - RB_IO_POINTER(io, fptr); - - if (rb_io_read_pending(fptr)) { - return Qtrue; - } + rb_io_t *fptr = NULL; + RB_IO_POINTER(io, fptr); + + if (rb_io_read_pending(fptr)) { + // This was the original behaviour: + if (return_io) return Qtrue; + // New behaviour always returns an event mask: + else return RB_INT2NUM(RUBY_IO_READABLE); + } } - return io_wait_event(io, events, timeout); + return io_wait_event(io, events, timeout, return_io); #endif } +#endif /* RUBY_IO_WAIT_METHODS */ + /* * IO wait methods */ @@ -386,6 +418,7 @@ Init_wait(void) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L418 rb_define_method(rb_cIO, "nread", io_nread, 0); rb_define_method(rb_cIO, "ready?", io_ready_p, 0); +#ifndef RUBY_IO_WAIT_METHODS rb_define_method(rb_cIO, "wait", io_wait, -1); rb_define_method(rb_cIO, "wait_readable", io_wait_readable, -1); @@ -393,4 +426,5 @@ Init_wait(void) https://github.com/ruby/ruby/blob/trunk/ext/io/wait/wait.c#L426 #ifdef HAVE_RB_IO_WAIT rb_define_method(rb_cIO, "wait_priority", io_wait_priority, -1); #endif +#endif } diff --git a/test/io/wait/test_io_wait.rb b/test/io/wait/test_io_wait.rb index 6b4722e1be..5d0f0bfd34 100644 --- a/test/io/wait/test_io_wait.rb +++ b/test/io/wait/test_io_wait.rb @@ -161,6 +161,34 @@ class TestIOWait < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/io/wait/test_io_wait.rb#L161 assert_equal @w, @w.wait(0.01, :read_write) end + def test_wait_mask_writable + omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE) + assert_equal IO::WRITABLE, @w.wait(IO::WRITABLE, 0) + end + + def test_wait_mask_readable + omit("Missing IO::READABLE!") unless IO.const_defined?(:READABLE) + @w.write("Hello World\n" * 3) + assert_equal IO::READABLE, @r.wait(IO::READABLE, 0) + + @r.gets + assert_equal IO::READABLE, @r.wait(IO::READABLE, 0) + end + + def test_wait_mask_zero + omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE) + assert_raises(ArgumentError) do + @w.wait(0, 0) + end + end + + def test_wait_mask_negative + omit("Missing IO::WRITABLE!") unless IO.const_defined?(:WRITABLE) + assert_raises(ArgumentError) do + @w.wait(-6, 0) + end + end + private def fill_pipe -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/