March 2024 - ruby-core - ml.ruby-lang.org

[ruby-core:117226] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.
by ioquatix (Samuel Williams) 19 Mar '24

19 Mar '24

Issue #19057 has been updated by ioquatix (Samuel Williams). The simplest option right now is to revert this change and try again later. @mame are there any other gems apart from `unicorn` you are concerned about? In other words, if `unicorn` makes a release, then you don't have a problem with this change right? @matz is this also your position? Is `unicorn` the only blocker that you are concerned about? ---------------------------------------- Feature #19057: Hide implementation of `rb_io_t`. https://bugs.ruby-lang.org/issues/19057#change-107314 * Author: ioquatix (Samuel Williams) * Status: Assigned * Assignee: ioquatix (Samuel Williams) * Target version: 3.4 ---------------------------------------- In order to make improvements to the IO implementation like <https://bugs.ruby-lang.org/issues/18455>, we need to add new fields to `struct rb_io_t`. By the way, ending types in `_t` is not recommended by POSIX, so I'm also trying to rename the internal implementation to drop `_t` where possible during this conversion. Anyway, we should try to hide the implementation of `struct rb_io`. Ideally, we don't expose any of it, but the problem is backwards compatibility. So, in order to remain backwards compatibility, we should expose some fields of `struct rb_io`, the most commonly used one is `fd` and `mode`, but several others are commonly used. There are many fields which should not be exposed because they are implementation details. ## Current proposal The current proposed change <https://github.com/ruby/ruby/pull/6511> creates two structs: ```c // include/ruby/io.h #ifndef RB_IO_T struct rb_io { int fd; // ... public fields ... }; #else struct rb_io; #endif // internal/io.h #define RB_IO_T struct rb_io { int fd; // ... public fields ... // ... private fields ... }; ``` However, we are not 100% confident this is safe according to the C specification. My experience is not sufficiently wide to say this is safe in practice, but it does look okay to both myself, and @Eregon + @tenderlovemaking have both given some kind of approval. That being said, maybe it's not safe. There are two alternatives: ## Hide all details We can make public `struct rb_io` completely invisible. ```c // include/ruby/io.h #define RB_IO_HIDDEN struct rb_io; int rb_ioptr_descriptor(struct rb_io *ioptr); // accessor for previously visible state. // internal/io.h struct rb_io { // ... all fields ... }; ``` This would only be forwards compatible, and code would need to feature detect like this: ```c #ifdef RB_IO_HIDDEN #define RB_IOPTR_DESCRIPTOR rb_ioptr_descriptor #else #define RB_IOPTR_DESCRIPTOR(ioptr) rb_ioptr_descriptor(ioptr) #endif ``` ## Nested public interface Alternatively, we can nest the public fields into the private struct: ```c // include/ruby/io.h struct rb_io_public { int fd; // ... public fields ... }; // internal/io.h #define RB_IO_T struct rb_io { struct rb_io_public public; // ... private fields ... }; ``` ## Considerations I personally think the "Hide all details" implementation is the best, but it's also the lest compatible. This is also what we are ultimately aiming for, whether we decide to take an intermediate "compatibility step" is up to us. I think "Nested public interface" is messy and introduces more complexity, but it might be slightly better defined than the "Current proposal" which might create undefined behaviour. That being said, all the tests are passing. -- https://bugs.ruby-lang.org/

1 0

[ruby-core:117224] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.
by matz (Yukihiro Matsumoto) 19 Mar '24

19 Mar '24

Issue #19057 has been updated by matz (Yukihiro Matsumoto). I agree with @mame. This change would break too many tests, apps, etc. We cannot accept the change at the moment. Can we be more conservative? Matz. ---------------------------------------- Feature #19057: Hide implementation of `rb_io_t`. https://bugs.ruby-lang.org/issues/19057#change-107310 * Author: ioquatix (Samuel Williams) * Status: Assigned * Assignee: ioquatix (Samuel Williams) * Target version: 3.4 ---------------------------------------- In order to make improvements to the IO implementation like <https://bugs.ruby-lang.org/issues/18455>, we need to add new fields to `struct rb_io_t`. By the way, ending types in `_t` is not recommended by POSIX, so I'm also trying to rename the internal implementation to drop `_t` where possible during this conversion. Anyway, we should try to hide the implementation of `struct rb_io`. Ideally, we don't expose any of it, but the problem is backwards compatibility. So, in order to remain backwards compatibility, we should expose some fields of `struct rb_io`, the most commonly used one is `fd` and `mode`, but several others are commonly used. There are many fields which should not be exposed because they are implementation details. ## Current proposal The current proposed change <https://github.com/ruby/ruby/pull/6511> creates two structs: ```c // include/ruby/io.h #ifndef RB_IO_T struct rb_io { int fd; // ... public fields ... }; #else struct rb_io; #endif // internal/io.h #define RB_IO_T struct rb_io { int fd; // ... public fields ... // ... private fields ... }; ``` However, we are not 100% confident this is safe according to the C specification. My experience is not sufficiently wide to say this is safe in practice, but it does look okay to both myself, and @Eregon + @tenderlovemaking have both given some kind of approval. That being said, maybe it's not safe. There are two alternatives: ## Hide all details We can make public `struct rb_io` completely invisible. ```c // include/ruby/io.h #define RB_IO_HIDDEN struct rb_io; int rb_ioptr_descriptor(struct rb_io *ioptr); // accessor for previously visible state. // internal/io.h struct rb_io { // ... all fields ... }; ``` This would only be forwards compatible, and code would need to feature detect like this: ```c #ifdef RB_IO_HIDDEN #define RB_IOPTR_DESCRIPTOR rb_ioptr_descriptor #else #define RB_IOPTR_DESCRIPTOR(ioptr) rb_ioptr_descriptor(ioptr) #endif ``` ## Nested public interface Alternatively, we can nest the public fields into the private struct: ```c // include/ruby/io.h struct rb_io_public { int fd; // ... public fields ... }; // internal/io.h #define RB_IO_T struct rb_io { struct rb_io_public public; // ... private fields ... }; ``` ## Considerations I personally think the "Hide all details" implementation is the best, but it's also the lest compatible. This is also what we are ultimately aiming for, whether we decide to take an intermediate "compatibility step" is up to us. I think "Nested public interface" is messy and introduces more complexity, but it might be slightly better defined than the "Current proposal" which might create undefined behaviour. That being said, all the tests are passing. -- https://bugs.ruby-lang.org/

1 0

[ruby-core:117212] [Ruby master Feature#20345] Add `--target-rbconfig` option to mkmf
by katei (Yuta Saito) 19 Mar '24

19 Mar '24

Issue #20345 has been reported by katei (Yuta Saito). ---------------------------------------- Feature #20345: Add `--target-rbconfig` option to mkmf https://bugs.ruby-lang.org/issues/20345 * Author: katei (Yuta Saito) * Status: Open ---------------------------------------- ## Motivation Today, CRuby runs on many platforms. But not all platforms are capable of running build tools (e.g. WebAssembly/WASI), so cross-target compilation against extensions libraries is essential for those platforms. We currently have 3 major mkmf users (`extconf.rb` consumers in in other words): 1. CRuby build system 2. rake-compiler 3. RubyGems [1] CRuby build system and [2] rake-compiler have their bespoke tricks to support cross compilation but [3] does not support cross compilation yet. So we are going to support cross-compilation in RubyGems to unlock the use of gems including non-precompiled extension libraries. However, introducing the same tricks to RubyGems to support cross compilation as well as the other two is not ideal and cannot handle some edge cases properly. Therefore, this proposal aims to add cross-compilation support in mkmf itself and remove the need for special tricks in mkmf users. Note that cross-compilation here includes: - Cross *platform* compilation: Build extension libraries for platform A on platform B. - Cross *ruby version* compilation: Build extension libraries for Ruby X with running mkmf.rb bundled with Ruby X on Ruby Y. ## Existing Solutions We currently have two solutions to cross-compile extension libraries, but both solutions are based on faking `rbconfig`. ### CRuby build system CRuby build system is capable for cross-compiling extension libraries for cross-platform and cross ruby version. The key trick here is that CRuby build system generates <platform>-fake.rb that fakes `RUBY_` constants like `RUBY_PLATFORM` and loads just built `rbconfig` describing Ruby version X for platform A and prevents loading `rbconfig` for Ruby version Y for platform B. As a result, this fakes the global `RbConfig` constant and mkmk generates Makefile using the faked `RbConfig`. ### rake-compiler rake-compiler also fakes `RbConfig` as well as CRuby build system does. One of the notable tricks here is that the faking script loads `resolv`, which expects the original `RUBY_PLATFORM`, at first and fake RbConfig after that. ```ruby # From https://github.com/rake-compiler/rake-compiler/blob/7357f9e917dae7935068778… # Pre-load resolver library before faking, in order to avoid error # "cannot load such file -- win32/resolv" when it is required later on. # See also: https://github.com/tjschuck/rake-compiler-dev-box/issues/5 require 'resolv' require 'rbconfig' ``` This has been introduced as a workaround but this indicates that the faking method cannot be generally applied. ## Problems Based on insights from the existing solutions, the problems here are: 1. There is no way to tell the target `RbConfig` to `mkmf` without polluting the global `RbConfig` constant. 2. There is no public API to retrieve the deployment target info, so existing `extconf.rb` assumes `::RbConfig` is the one. ## Proposal I propose adding those interfaces to `mkmf`: 1. `--target-rbconfig` option to override the RbConfig used for generating Makefiles without replacing the global top-level `RbConfig` module. 2. `MakeMakefile::RbConfig` constant to access the RbConfig for the target platform. By default, it's an alias of top-level `RbConfig`. If `--target-rbconfig` is given, it points to the specified `RbConfig` definition. ```console= $ ruby extconf.rb --target-rbconfig path/to/rbconfig.rb ``` ```ruby require "mkmf" system( "./libyaml/configure", # Before: # "--host=#{RbConfig::CONFIG['host']}", "--host=#{MakeMakefile::RbConfig::CONFIG['host']}", ... ) # Before: # case RUBY_PLATFORM case MakeMakefile::RbConfig::CONFIG['platform'] when /mswin|mingw|bccwin/ ... when /linux/ ... end create_makefile("psych") ``` Extension library authors who want to support cross-compilation just need to replace their use of some constants in `extconf.rb` that assume the config describes the deployment target. Here is the list of faked constant variables and corresponding representations compatible with cross-compilation. | Before | After (to make the ext x-compile ready) | |:------|:-----| |`RbConfig` | `MakeMakefile::RbConfig` | |`RUBY_PLATFORM` | `MakeMakefile::RbConfig::CONFIG["platform"]` | |`RUBY_VERSION` | `MakeMakefile::RbConfig::expand("$(MAJOR).$(MINOR).$(TEENY)")` | |`RUBY_DESCRIPTION` | No corresponding config entry | ## Compatibility This is a completely additive change, so I expect there is no compatibility issues for existing `extconf.rb`. Note that migrating `RbConfig` to `MakeMakefile::RbConfig` does not break existing faked `RbConfig` based cross-compilation because `MakeMakefile::RbConfig` is an alias of `::RbConfig` by default and it's the faked config describing the deployment target in this scenario. Also extension library authors who want to support cross-compilation and want to keep build with older Ruby before this change can include the following snippet at the beginning of `extconf.rb`: ```ruby= MakeMakefile::RbConfig ||= RbConfig ``` ## Implementation Literally a few lines of changes: https://github.com/kateinoigakukun/ruby/commit/9f3090c26ae1e5712dee702c19ba… ## Evaluation I ported nokogiri gem, which has [1k lines of `extconf.rb`](https://github.com/sparklemotion/nokogiri/blob/v1.16.3/ext/nokogiri/extconf.rb) and several platform specific branches, to WebAssembly/WASI with this change, and the new API was enough to satisfy the cross-compilation scenario. -- https://bugs.ruby-lang.org/

5 8

[ruby-core:116827] [Ruby master Feature#20276] Introduce Fiber interfaces for Ractors
by forthoney (Seong-Heon Jung) 18 Mar '24

18 Mar '24

Issue #20276 has been reported by forthoney (Seong-Heon Jung). ---------------------------------------- Feature #20276: Introduce Fiber interfaces for Ractors https://bugs.ruby-lang.org/issues/20276 * Author: forthoney (Seong-Heon Jung) * Status: Open * Priority: Normal ---------------------------------------- ## Motivation I am trying to build a web server with Ractors. The lifecycle for a request in the current implementation is 1. main ractor buffers request 2. main ractor sends request to worker ractor 3. worker ractor sends response to main ractor 4. main ractor writes response 5. repeat The main ractor utilizes the Async gem (specifically async-http) to handle connections concurrently, meaning each request is handled on a separate fiber. The issue I am running into is after I send a request to a worker ractor, I need to do a blocking wait until I receive a response. While I am waiting for the response, I cannot take any more connections. ## Solution If the fiber scheduler had a hook for `Ractor.receive` or `Ractor#take` (both of which are blocking), the main ractor can send the message, handle other connections while the worker processes the request. When the worker produces a message, it will then take the reqeust and write it in the socket. -- https://bugs.ruby-lang.org/

3 7

[ruby-core:117088] [Ruby master Feature#20329] Clean up `--dump` sub-options
by nobu (Nobuyoshi Nakada) 18 Mar '24

18 Mar '24

Issue #20329 has been reported by nobu (Nobuyoshi Nakada). ---------------------------------------- Feature #20329: Clean up `--dump` sub-options https://bugs.ruby-lang.org/issues/20329 * Author: nobu (Nobuyoshi Nakada) * Status: Open ---------------------------------------- Currently we have 5 options for `--dump` command line option. * insns * insns_without_opt * yydebug(+error-tolerant) * parsetree(+error-tolerant) * parsetree_with_comment(+error-tolerant) Among these, `insns_without_opt` is a variant of `insns`, and `parsetree_with_comment` is a variant of `parsetree`. However, there is now another way to specify variants (e.g. `+error-tolerant`). How about unifying the two so that the former can also be specified in the same form, such as `--dump=parsetree+comment+error-tolerant`? It also will be able to abbreviate as parse+comm+err` or more. -- https://bugs.ruby-lang.org/

3 5

[ruby-core:117208] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.
by ioquatix (Samuel Williams) 18 Mar '24

18 Mar '24

Issue #19057 has been updated by ioquatix (Samuel Williams). I don't use `unicorn` so thanks for the clarification. Maybe someone who does know, can write some documentation about the current state of affairs and how to use the current head. My trivial `gem build` and `gem install` seemed to work, but I didn't test it beyond that. ---------------------------------------- Feature #19057: Hide implementation of `rb_io_t`. https://bugs.ruby-lang.org/issues/19057#change-107297 * Author: ioquatix (Samuel Williams) * Status: Assigned * Assignee: ioquatix (Samuel Williams) * Target version: 3.4 ---------------------------------------- In order to make improvements to the IO implementation like <https://bugs.ruby-lang.org/issues/18455>, we need to add new fields to `struct rb_io_t`. By the way, ending types in `_t` is not recommended by POSIX, so I'm also trying to rename the internal implementation to drop `_t` where possible during this conversion. Anyway, we should try to hide the implementation of `struct rb_io`. Ideally, we don't expose any of it, but the problem is backwards compatibility. So, in order to remain backwards compatibility, we should expose some fields of `struct rb_io`, the most commonly used one is `fd` and `mode`, but several others are commonly used. There are many fields which should not be exposed because they are implementation details. ## Current proposal The current proposed change <https://github.com/ruby/ruby/pull/6511> creates two structs: ```c // include/ruby/io.h #ifndef RB_IO_T struct rb_io { int fd; // ... public fields ... }; #else struct rb_io; #endif // internal/io.h #define RB_IO_T struct rb_io { int fd; // ... public fields ... // ... private fields ... }; ``` However, we are not 100% confident this is safe according to the C specification. My experience is not sufficiently wide to say this is safe in practice, but it does look okay to both myself, and @Eregon + @tenderlovemaking have both given some kind of approval. That being said, maybe it's not safe. There are two alternatives: ## Hide all details We can make public `struct rb_io` completely invisible. ```c // include/ruby/io.h #define RB_IO_HIDDEN struct rb_io; int rb_ioptr_descriptor(struct rb_io *ioptr); // accessor for previously visible state. // internal/io.h struct rb_io { // ... all fields ... }; ``` This would only be forwards compatible, and code would need to feature detect like this: ```c #ifdef RB_IO_HIDDEN #define RB_IOPTR_DESCRIPTOR rb_ioptr_descriptor #else #define RB_IOPTR_DESCRIPTOR(ioptr) rb_ioptr_descriptor(ioptr) #endif ``` ## Nested public interface Alternatively, we can nest the public fields into the private struct: ```c // include/ruby/io.h struct rb_io_public { int fd; // ... public fields ... }; // internal/io.h #define RB_IO_T struct rb_io { struct rb_io_public public; // ... private fields ... }; ``` ## Considerations I personally think the "Hide all details" implementation is the best, but it's also the lest compatible. This is also what we are ultimately aiming for, whether we decide to take an intermediate "compatibility step" is up to us. I think "Nested public interface" is messy and introduces more complexity, but it might be slightly better defined than the "Current proposal" which might create undefined behaviour. That being said, all the tests are passing. -- https://bugs.ruby-lang.org/

1 0

[ruby-core:117207] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.
by byroot (Jean Boussier) 18 Mar '24

18 Mar '24

Issue #19057 has been updated by byroot (Jean Boussier). > are you able to point it at the git head? You can't just do that because Unicorn has a bunch of files that need to be generated and are not compiled. So you need a custom fork: https://github.com/k0kubun/unicorn/commit/6215d4cad7a964bf4b8bdef48edadf334… ---------------------------------------- Feature #19057: Hide implementation of `rb_io_t`. https://bugs.ruby-lang.org/issues/19057#change-107296 * Author: ioquatix (Samuel Williams) * Status: Assigned * Assignee: ioquatix (Samuel Williams) * Target version: 3.4 ---------------------------------------- In order to make improvements to the IO implementation like <https://bugs.ruby-lang.org/issues/18455>, we need to add new fields to `struct rb_io_t`. By the way, ending types in `_t` is not recommended by POSIX, so I'm also trying to rename the internal implementation to drop `_t` where possible during this conversion. Anyway, we should try to hide the implementation of `struct rb_io`. Ideally, we don't expose any of it, but the problem is backwards compatibility. So, in order to remain backwards compatibility, we should expose some fields of `struct rb_io`, the most commonly used one is `fd` and `mode`, but several others are commonly used. There are many fields which should not be exposed because they are implementation details. ## Current proposal The current proposed change <https://github.com/ruby/ruby/pull/6511> creates two structs: ```c // include/ruby/io.h #ifndef RB_IO_T struct rb_io { int fd; // ... public fields ... }; #else struct rb_io; #endif // internal/io.h #define RB_IO_T struct rb_io { int fd; // ... public fields ... // ... private fields ... }; ``` However, we are not 100% confident this is safe according to the C specification. My experience is not sufficiently wide to say this is safe in practice, but it does look okay to both myself, and @Eregon + @tenderlovemaking have both given some kind of approval. That being said, maybe it's not safe. There are two alternatives: ## Hide all details We can make public `struct rb_io` completely invisible. ```c // include/ruby/io.h #define RB_IO_HIDDEN struct rb_io; int rb_ioptr_descriptor(struct rb_io *ioptr); // accessor for previously visible state. // internal/io.h struct rb_io { // ... all fields ... }; ``` This would only be forwards compatible, and code would need to feature detect like this: ```c #ifdef RB_IO_HIDDEN #define RB_IOPTR_DESCRIPTOR rb_ioptr_descriptor #else #define RB_IOPTR_DESCRIPTOR(ioptr) rb_ioptr_descriptor(ioptr) #endif ``` ## Nested public interface Alternatively, we can nest the public fields into the private struct: ```c // include/ruby/io.h struct rb_io_public { int fd; // ... public fields ... }; // internal/io.h #define RB_IO_T struct rb_io { struct rb_io_public public; // ... private fields ... }; ``` ## Considerations I personally think the "Hide all details" implementation is the best, but it's also the lest compatible. This is also what we are ultimately aiming for, whether we decide to take an intermediate "compatibility step" is up to us. I think "Nested public interface" is messy and introduces more complexity, but it might be slightly better defined than the "Current proposal" which might create undefined behaviour. That being said, all the tests are passing. -- https://bugs.ruby-lang.org/

1 0

[ruby-core:117206] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.
by ioquatix (Samuel Williams) 18 Mar '24

18 Mar '24

Issue #19057 has been updated by ioquatix (Samuel Williams). @mame are you able to point it at the git head? e.g. ```ruby gem "unicorn", git: "https://yhbt.net/unicorn.git" # or gem "unicorn", git: "https://github.com/socketry/unicorn.git" ``` That should get things moving again. I hope Eric will release an updated `unicorn` soon. However, if that doesn't happen, I see a couple of options: - We can release `unicorn2` gem. - Companies with a vested interest could fork unicorn for internal use. - Companies could contact Eric and offer incentives for him to make a release. ---------------------------------------- Feature #19057: Hide implementation of `rb_io_t`. https://bugs.ruby-lang.org/issues/19057#change-107295 * Author: ioquatix (Samuel Williams) * Status: Assigned * Assignee: ioquatix (Samuel Williams) * Target version: 3.4 ---------------------------------------- In order to make improvements to the IO implementation like <https://bugs.ruby-lang.org/issues/18455>, we need to add new fields to `struct rb_io_t`. By the way, ending types in `_t` is not recommended by POSIX, so I'm also trying to rename the internal implementation to drop `_t` where possible during this conversion. Anyway, we should try to hide the implementation of `struct rb_io`. Ideally, we don't expose any of it, but the problem is backwards compatibility. So, in order to remain backwards compatibility, we should expose some fields of `struct rb_io`, the most commonly used one is `fd` and `mode`, but several others are commonly used. There are many fields which should not be exposed because they are implementation details. ## Current proposal The current proposed change <https://github.com/ruby/ruby/pull/6511> creates two structs: ```c // include/ruby/io.h #ifndef RB_IO_T struct rb_io { int fd; // ... public fields ... }; #else struct rb_io; #endif // internal/io.h #define RB_IO_T struct rb_io { int fd; // ... public fields ... // ... private fields ... }; ``` However, we are not 100% confident this is safe according to the C specification. My experience is not sufficiently wide to say this is safe in practice, but it does look okay to both myself, and @Eregon + @tenderlovemaking have both given some kind of approval. That being said, maybe it's not safe. There are two alternatives: ## Hide all details We can make public `struct rb_io` completely invisible. ```c // include/ruby/io.h #define RB_IO_HIDDEN struct rb_io; int rb_ioptr_descriptor(struct rb_io *ioptr); // accessor for previously visible state. // internal/io.h struct rb_io { // ... all fields ... }; ``` This would only be forwards compatible, and code would need to feature detect like this: ```c #ifdef RB_IO_HIDDEN #define RB_IOPTR_DESCRIPTOR rb_ioptr_descriptor #else #define RB_IOPTR_DESCRIPTOR(ioptr) rb_ioptr_descriptor(ioptr) #endif ``` ## Nested public interface Alternatively, we can nest the public fields into the private struct: ```c // include/ruby/io.h struct rb_io_public { int fd; // ... public fields ... }; // internal/io.h #define RB_IO_T struct rb_io { struct rb_io_public public; // ... private fields ... }; ``` ## Considerations I personally think the "Hide all details" implementation is the best, but it's also the lest compatible. This is also what we are ultimately aiming for, whether we decide to take an intermediate "compatibility step" is up to us. I think "Nested public interface" is messy and introduces more complexity, but it might be slightly better defined than the "Current proposal" which might create undefined behaviour. That being said, all the tests are passing. -- https://bugs.ruby-lang.org/

1 0

[ruby-core:117204] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.
by mame (Yusuke Endoh) 18 Mar '24

18 Mar '24

Issue #19057 has been updated by mame (Yusuke Endoh). This change has broken our internal CI. Because our CI contributes to assure the quality of Ruby master, it is a shame that it will not work until the release of unicorn, which we do not know when (and whether) it will be released. I hope to postpone the change at least until unicorn etc. is released. ---------------------------------------- Feature #19057: Hide implementation of `rb_io_t`. https://bugs.ruby-lang.org/issues/19057#change-107293 * Author: ioquatix (Samuel Williams) * Status: Assigned * Assignee: ioquatix (Samuel Williams) * Target version: 3.4 ---------------------------------------- In order to make improvements to the IO implementation like <https://bugs.ruby-lang.org/issues/18455>, we need to add new fields to `struct rb_io_t`. By the way, ending types in `_t` is not recommended by POSIX, so I'm also trying to rename the internal implementation to drop `_t` where possible during this conversion. Anyway, we should try to hide the implementation of `struct rb_io`. Ideally, we don't expose any of it, but the problem is backwards compatibility. So, in order to remain backwards compatibility, we should expose some fields of `struct rb_io`, the most commonly used one is `fd` and `mode`, but several others are commonly used. There are many fields which should not be exposed because they are implementation details. ## Current proposal The current proposed change <https://github.com/ruby/ruby/pull/6511> creates two structs: ```c // include/ruby/io.h #ifndef RB_IO_T struct rb_io { int fd; // ... public fields ... }; #else struct rb_io; #endif // internal/io.h #define RB_IO_T struct rb_io { int fd; // ... public fields ... // ... private fields ... }; ``` However, we are not 100% confident this is safe according to the C specification. My experience is not sufficiently wide to say this is safe in practice, but it does look okay to both myself, and @Eregon + @tenderlovemaking have both given some kind of approval. That being said, maybe it's not safe. There are two alternatives: ## Hide all details We can make public `struct rb_io` completely invisible. ```c // include/ruby/io.h #define RB_IO_HIDDEN struct rb_io; int rb_ioptr_descriptor(struct rb_io *ioptr); // accessor for previously visible state. // internal/io.h struct rb_io { // ... all fields ... }; ``` This would only be forwards compatible, and code would need to feature detect like this: ```c #ifdef RB_IO_HIDDEN #define RB_IOPTR_DESCRIPTOR rb_ioptr_descriptor #else #define RB_IOPTR_DESCRIPTOR(ioptr) rb_ioptr_descriptor(ioptr) #endif ``` ## Nested public interface Alternatively, we can nest the public fields into the private struct: ```c // include/ruby/io.h struct rb_io_public { int fd; // ... public fields ... }; // internal/io.h #define RB_IO_T struct rb_io { struct rb_io_public public; // ... private fields ... }; ``` ## Considerations I personally think the "Hide all details" implementation is the best, but it's also the lest compatible. This is also what we are ultimately aiming for, whether we decide to take an intermediate "compatibility step" is up to us. I think "Nested public interface" is messy and introduces more complexity, but it might be slightly better defined than the "Current proposal" which might create undefined behaviour. That being said, all the tests are passing. -- https://bugs.ruby-lang.org/

1 0

[ruby-core:117202] [Ruby master Bug#20343] Ripper.sexp in Ruby 3.4 returns nil
by koic (Koichi ITO) 17 Mar '24

17 Mar '24

Issue #20343 has been reported by koic (Koichi ITO). ---------------------------------------- Bug #20343: Ripper.sexp in Ruby 3.4 returns nil https://bugs.ruby-lang.org/issues/20343 * Author: koic (Koichi ITO) * Status: Open * ruby -v: ruby 3.4.0dev (2024-03-17T10:09:37Z master 5fd6b461c7) [x86_64-darwin23] * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- In Ruby 3.4.0's Ripper, there is the following incompatible behavior: ## Expected (Ruby 3.3) It is expected that tokens will be returned. ```console $ ruby -rripper -ve "p Ripper.sexp('foo[bar, &baz] += 1')" ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] [:program, [[:opassign, [:aref_field, [:vcall, [:@ident, "foo", [1, 0]]], [:args_add_block, [[:vcall, [:@ident, "bar", [1, 4]]]], [:vcall, [:@ident, "baz", [1, 10]]]]], [:@op, "+=", [1, 15]], [:@int, "1", [1, 18]]]]] $ ruby -rripper -ve "p Ripper.sexp_raw('foo[bar, &baz] += 1')" ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] [:program, [:stmts_add, [:stmts_new], [:opassign, [:aref_field, [:vcall, [:@ident, "foo", [1, 0]]], [:args_add_block, [:args_add, [:args_new], [:vcall, [:@ident, "bar", [1, 4]]]], [:vcall, [:@ident, "baz", [1, 10]]]]], [:@op, "+=", [1, 15]], [:@int, "1", [1, 18]]]]] ``` ## Actual (Ruby 3.4) `nil` is returned. ```console $ ruby -rripper -ve "p Ripper.sexp('foo[bar, &baz] += 1')" ruby 3.4.0dev (2024-03-17T10:09:37Z master 5fd6b461c7) [x86_64-darwin23] nil $ ruby -rripper -ve "p Ripper.sexp_raw('foo[bar, &baz] += 1')" ruby 3.4.0dev (2024-03-17T10:09:37Z master 5fd6b461c7) [x86_64-darwin23] nil ``` The same applies to the following cases. ```ruby foo[bar, &baz] += 1 foo[bar, &baz] ||= 1 foo[bar, &baz] &&= 1 foo.foo[bar, &baz] += 1 foo.foo[bar, &baz] ||= 1 foo.foo[bar, &baz] &&= 1 ``` This was discovered in a failure of Prism's CI: https://github.com/ruby/prism/actions/runs/8317131929/job/22757514162?pr=26… -- https://bugs.ruby-lang.org/

2 1