[ruby-core:110300] [Ruby master Bug#19057] Hide implementation of `rb_io_t`.

[ruby-core:110300] [Ruby master Bug#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams)]

Issue #19057 has been reported by ioquatix (Samuel Williams).

----------------------------------------
Bug #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
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/

[ruby-core:110301] [Ruby master Bug#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams)]

Issue #19057 has been updated by ioquatix (Samuel Williams).


Twitter discussion with @alanwu: <https://twitter.com/alanwusx/status/1581086330259668992>

----------------------------------------
Bug #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-99586

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
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/

[ruby-core:110309] [Ruby master Bug#19057] Hide implementation of `rb_io_t`.

[Eregon (Benoit Daloze)]

Issue #19057 has been updated by Eregon (Benoit Daloze).


I support the `Hide all details` approach.
With one tweak: not `int rb_ioptr_descriptor(struct rb_io *ioptr);` but `int rb_io_descriptor(VALUE io);`.
The goal is to not expose the struct in the C API, so let's not expose it (even if it becomes just an opaque pointer).

Not exposing structs in the C API makes it much easier to support the C API and evolve it.
For example it would make it easier to implement the C API with JNI or so in TruffleRuby, similar to HPy efforts in Python.
Having to emulate structs on GraalVM Sulong is additional complexity and makes C extensions slower to warm up (https://medium.com/graalvm/porting-matplotlib-from-c-api-to-hpy-aa32faa1f0b5).

If we go directly to the `Hide all details` approach it might be more painful to transition though, i.e., more of a breaking change.

Maybe we can simply deprecate `GetOpenFile` (which BTW does not have a proper `RB_` prefix so more reason to deprecate), or deprecate `rb_io_t` itself if that's possible?
And then of course we'd already add `int rb_io_descriptor(VALUE io);` etc.
So for 1-2 releases we wouldn't break anything, just deprecation warnings and let more time for people to switch to `int rb_io_descriptor(VALUE io);` etc.

----------------------------------------
Bug #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-99600

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
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/

[ruby-core:110348] [Ruby master Bug#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams)]

Issue #19057 has been updated by ioquatix (Samuel Williams).


I agree with `Hide all details` but I'm more willing to compromise on the interface.

> With one tweak: not int rb_ioptr_descriptor(struct rb_io *ioptr); but int rb_io_descriptor(VALUE io);.

Maybe it's CRuby problem, but there is a performance issue with such an implementation which has to extract and follow the pointer to the struct over and over again. If we can accept that, I'm okay with your proposal, but it's also a significant cost to compatibility.

----------------------------------------
Bug #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-99639

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
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/

[ruby-core:113678] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[kjtsanaktsidis (KJ Tsanaktsidis) via ruby-core]

Issue #19057 has been updated by kjtsanaktsidis (KJ Tsanaktsidis).


I did a bit of research on this topic this evening.

Firstly, some technical notes r.e. undefined behaviour.

My understanding is that "Current proposal" is really undefined behaviour. This is in the C standard, section 6.5.2.3, point 6 (http://www.open-std.org/jtc1/sc22/wg14/www/abq/c17_updated_proposed_fdis.pdf)

> One special guarantee is made in order to simplify the use of unions: if a union contains several
> structures that share a common initial sequence (see below), and if the union object currently contains
> one of these structures, it is permitted to inspect the common initial part of any of them anywhere
> that a declaration of the completed type of the union is visible. Two structures share a common initial
> sequence if corresponding members have compatible types (and, for bit-fields, the same widths) for a
> sequence of one or more initial members

It seems that this "common initial sequence" rule is really only for structures accessed through unions. Another fun gotcha is that, even if you _do_ do all the accesses through a union, the compiler can assume that `rb_io_public_t *` and `rb_io_private_t *` should never alias each other (which is the exact opposite of what you want). E.g. - this program has a different result depending on whether optimizations are on or not - http://coliru.stacked-crooked.com/a/b57c8dd9e2ef3a02

I think to be technically correct, we would need to go for the "Nested public interface" approach - structs _can_ be converted to a pointer to their first field according to the C standard (although I guess the _reverse_ cast from `rb_io_public_t *` back to `rb_io_private_t *` would be UB?)

------------

I don't think this matters though, because I agree that, looking at the contents of `rb_io_t`, almost none of this should be public API, and we should strive to encapsulate it fully, like "Hide all details" (or like @Eregon suggested too).

I did a Github code search for usages of `rb_io_t`, and _all_ of the usages of it I could find basically fell into this pattern.

```
VALUE some_io = /* from somewhere */;
rb_io_t *fptr;
GetOpenFile(some_io, fptr)

// call an io.h method that takes rb_io_t *
rb_io_set_nonblock(fptr);
// or read the fd member and do something with that
do_something_external_with_fd(fptr->fd);
```

I think we could avoid 99% of the breakage, and still hide all the implementation details of rb_io_t, by basically redefining `rb_io_t` to contain just the FD, and define a new internal, opaque type for internal IO use. We would do this like so:

Firstly, `struct RFile` is currently in include/ruby/internal/core/rfile.h as:

```
struct rb_io_t;
struct RFile {
    struct RBasic basic;
    struct rb_io_t *fptr;
};
```

There are two spare words in there, and we would use one to add a pointer to a new structure. The definition of that structure would _not_ be provided in public headers anyhere. So, we would change RFile in include/ruby/internal/core/rfile.h to be:

```
struct rb_io_t;
struct rb_io_impl;
struct RFile {
    struct RBasic basic;
    struct rb_io_t *fptr;
    struct rb_io_impl *impl;
};
```


Then, we would change the definition of `struct rb_io_t` in include/ruby/io.h to be:

```
typedef struct rb_io_t {
    VALUE self;
    int fd;
    // _everything_ else is removed
} rb_io_t;
```

We would move all of its current contents to a new struct in internal/io.h:

```
struct rb_io_impl {
    // All the juicy implementation details go here - _except_ no need to duplicate `self` and `fd`.
};
```

The current definition of `GetOpenFile` essentially does `RFILE(obj)->fptr`, so that would return the newly-slimmed-down public `rb_io_t`. Attempts to read the file descriptor off that would continue to work.

Other methods in include/ruby/io.h which take a `rb_io_t *` as an argument, like e.g. `rb_io_set_nonblock`, would do `RFILE(fptr->self)->impl` to get access to the implementation details struct. We could also define variants of these which took the `VALUE` instead of the `rb_io_t *` to avoid the pointer-chase of `fptr->self` if desired.

The downside of this approach is that, as you identified, it costs an extra indirection in the implementation of most IO methods to obtain the impl from the VALUE. But, the benefits are that we get almost total encapsulation whilst maintaining backwards compatibility with almost all existing code. We can also shuffle fields between the private & public structs depending on observed compatibility issues found in the wild too.


----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103322

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113680] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eregon (Benoit Daloze) via ruby-core]

Issue #19057 has been updated by Eregon (Benoit Daloze).


ioquatix (Samuel Williams) wrote in #note-3:
> Maybe it's CRuby problem, but there is a performance issue with such an implementation which has to extract and follow the pointer to the struct over and over again.

It's pretty rare to access multiple fields of `rb_io_t` in code I have seen, so then performance-wise it is the same if accessing a single field.
When accessing two fields it would be indeed an extra indirection, although I highly doubt this would be measurable overhead.

Coming from the other way, on all Ruby implementations besides CRuby, exposing `struct rb_io *ioptr` would mean allocating a struct just to support those accesses, for every IO instance, possibly lazily. That's a much much bigger overhead than an extra pointer indirection.

As HPy shows, a fast and portable C API does not expose structs.


----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103324

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113681] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


Given the discussion here, I just want to aim for "Hide all the details".

Thanks everyone, the discussion has been most helpful.

I especially like the discussions around undefined behaviour.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103325

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113689] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ianks (Ian Ker-Seymer) via ruby-core]

Issue #19057 has been updated by ianks (Ian Ker-Seymer).


> With one tweak: not int rb_ioptr_descriptor(struct rb_io *ioptr); but int rb_io_descriptor(VALUE io);.
> The goal is to not expose the struct in the C API, so let's not expose it (even if it becomes just an opaque pointer).

I have mixed feelings about this. On one hand, it does successfully hide the implementation. On the other, casting pointers to unsigned ints has implications for pointer provenance models. Would be nice to get the opaqueness without the int-cast.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103334

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113696] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eregon (Benoit Daloze) via ruby-core]

Issue #19057 has been updated by Eregon (Benoit Daloze).


ianks (Ian Ker-Seymer) wrote in #note-8:
> > With one tweak: not int rb_ioptr_descriptor(struct rb_io *ioptr); but int rb_io_descriptor(VALUE io);.
> > The goal is to not expose the struct in the C API, so let's not expose it (even if it becomes just an opaque pointer).
> 
> On the other, casting pointers to unsigned ints has implications for pointer provenance models.

What do you mean? `struct RBasic*` to `VALUE` or vice-versa?
Almost every C API function does that, so that seems a separate issue.

Although maybe the above is not clear? `int rb_io_descriptor(VALUE io);` would be called with the Ruby (IO) object (like almost every other C API function), not with a pointer (opaque or not) to the `rb_io_t` struct.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103342

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113698] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).

Status changed from Open to Closed

https://github.com/ruby/ruby/pull/6511 was merged.

In addition, the following related PRs were merged:

- https://github.com/ruby/etc/pull/26
- https://github.com/ruby/io-wait/pull/25
- https://github.com/ruby/io-console/pull/43

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103344

* Author: ioquatix (Samuel Williams)
* Status: Closed
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113723] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


Unfortunately this was reverted due to some extensions no longer compiling.

- https://bugs.ruby-lang.org/issues/19704

I think it's expected that some dependencies using internal details of `rb_io_t` should be fixed.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103373

* Author: ioquatix (Samuel Williams)
* Status: Closed
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113728] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eregon (Benoit Daloze) via ruby-core]

Issue #19057 has been updated by Eregon (Benoit Daloze).


I am not sure there is an easier path here, IMO it's OK for some extensions to break while they test against ruby-head, and to adapt to it (and we are not close to December).
It would be nicer to deprecate first, but I am not sure how feasible is that.

It seems maybe GCC & Clang might support deprecating struct members: https://stackoverflow.com/questions/22823477/c-portable-way-to-deprecate-struct-members
Then we could deprecate every member of `rb_io_t`.
That seems useful, even if the warning only works on those compilers, most people would see it.

That said, even with deprecations for one release I'm pretty sure not all extensions using rb_io_t members will have migrated, so there will be some breakage anyway, just like for every deprecated thing.
But if extensions didn't migrate for more than a year, then it's really on them if they did not address it in time.

Another way could be to deprecate `GetOpenFile` (or the whole struct type if that's possible).
But that seems difficult because many `rb_io_*` functions take a rb_io_t*, e.g., `FILE *rb_io_stdio_file(rb_io_t *fptr);`, so we would then need to add a variant taking `VALUE io` for all of these (with another name).
There is less value in that, because if `rb_io_t` is opaque these functions are not really problematic, e.g. TruffleRuby could just return and cast the `io` for `GetOpenFile(io)`, and it would require larger efforts to migrate.
The last paragraph of https://bugs.ruby-lang.org/issues/19057#note-2 was mentioning similar ideas about deprecation.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103379

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113731] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


I'm in favour of eventually deprecating `rb_io_t` and I think that means anything related to it.

I think deprecations can make things a little easier but I'm not against breaking things that are out of our control.

I'm happy to fix things as they come up. Today we fixed nio4r, and polyphony, along with `readline-ext` and the other `io-*` gems.

I agree, once we stop exposing `rb_io_t`, even CRuby can take the same approach (just return the `IO` value). Whether we want to or not is another question, but I imagine only a few `VALLUE io` functions are missing now.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103381

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113802] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


Okay, here is a PR to introduce deprecations: https://github.com/ruby/ruby/pull/7916

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103448

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113844] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[byroot (Jean Boussier) via ruby-core]

Issue #19057 has been updated by byroot (Jean Boussier).


@ioquatix I'm not sure which change exactly is the cause, but it appears that the recent `rb_io_t` changes broke `raindrops`


```
current directory: /usr/local/lib/ruby/gems/3.3.0+0/gems/raindrops-0.20.1/ext/raindrops
make DESTDIR\= sitearchdir\=./.gem.20230609-25-giurc0 sitelibdir\=./.gem.20230609-25-giurc0 clean

current directory: /usr/local/lib/ruby/gems/3.3.0+0/gems/raindrops-0.20.1/ext/raindrops
make DESTDIR\= sitearchdir\=./.gem.20230609-25-giurc0 sitelibdir\=./.gem.20230609-25-giurc0
compiling linux_inet_diag.c
In file included from /usr/include/x86_64-linux-gnu/bits/libc-header-start.h:33,
                 from /usr/include/stdio.h:27,
                 from /usr/local/include/ruby-3.3.0+0/ruby/defines.h:16,
                 from /usr/local/include/ruby-3.3.0+0/ruby/ruby.h:25,
                 from /usr/local/include/ruby-3.3.0+0/ruby.h:38,
                 from linux_inet_diag.c:1:
/usr/include/features.h:187:3: warning: #warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE" [-Wcpp]
  187 | # warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE"
      |   ^~~~~~~
In file included from linux_inet_diag.c:4:
my_fileno.h:9:7: warning: "HAVE_RB_IO_T" is not defined, evaluates to 0 [-Wundef]
    9 | #if ! HAVE_RB_IO_T
      |       ^~~~~~~~~~~~
my_fileno.h:16:8: warning: "HAVE_RB_IO_T" is not defined, evaluates to 0 [-Wundef]
   16 | #  if !HAVE_RB_IO_T || (RUBY_VERSION_MAJOR == 1 && RUBY_VERSION_MINOR == 8)
      |        ^~~~~~~~~~~~
my_fileno.h: In function ‘my_fileno’:
my_fileno.h:10:19: error: unknown type name ‘OpenFile’; did you mean ‘GetOpenFile’?
   10 | #  define rb_io_t OpenFile
      |                   ^~~~~~~~
my_fileno.h:25:2: note: in expansion of macro ‘rb_io_t’
   25 |  rb_io_t *fptr;
      |  ^~~~~~~
In file included from my_fileno.h:3,
                 from linux_inet_diag.c:4:
/usr/local/include/ruby-3.3.0+0/ruby/io.h:395:55: warning: assignment to ‘int *’ from incompatible pointer type ‘struct rb_io *’ [-Wincompatible-pointer-types]
  395 | #define RB_IO_POINTER(obj,fp) rb_io_check_closed((fp) = RFILE(rb_io_taint_check(obj))->fptr)
      |                                                       ^
/usr/local/include/ruby-3.3.0+0/ruby/io.h:401:21: note: in expansion of macro ‘RB_IO_POINTER’
  401 | #define GetOpenFile RB_IO_POINTER
      |                     ^~~~~~~~~~~~~
my_fileno.h:30:2: note: in expansion of macro ‘GetOpenFile’
   30 |  GetOpenFile(io, fptr);
      |  ^~~~~~~~~~~
/usr/local/include/ruby-3.3.0+0/ruby/io.h:395:55: warning: passing argument 1 of ‘rb_io_check_closed’ from incompatible pointer type [-Wincompatible-pointer-types]
  395 | #define RB_IO_POINTER(obj,fp) rb_io_check_closed((fp) = RFILE(rb_io_taint_check(obj))->fptr)
      |                                                  ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                       |
      |                                                       int *
/usr/local/include/ruby-3.3.0+0/ruby/io.h:401:21: note: in expansion of macro ‘RB_IO_POINTER’
  401 | #define GetOpenFile RB_IO_POINTER
      |                     ^~~~~~~~~~~~~
my_fileno.h:30:2: note: in expansion of macro ‘GetOpenFile’
   30 |  GetOpenFile(io, fptr);
      |  ^~~~~~~~~~~
/usr/local/include/ruby-3.3.0+0/ruby/io.h:638:34: note: expected ‘rb_io_t *’ {aka ‘struct rb_io *’} but argument is of type ‘int *’
  638 | void rb_io_check_closed(rb_io_t *fptr);
      |                         ~~~~~~~~~^~~~
In file included from linux_inet_diag.c:4:
my_fileno.h:17:41: error: request for member ‘f’ in something not a structure or union
   17 | #    define FPTR_TO_FD(fptr) fileno(fptr->f)
      |                                         ^~
my_fileno.h:31:7: note: in expansion of macro ‘FPTR_TO_FD’
   31 |  fd = FPTR_TO_FD(fptr);
      |       ^~~~~~~~~~
linux_inet_diag.c: At top level:
cc1: warning: unrecognized command line option ‘-Wno-self-assign’
cc1: warning: unrecognized command line option ‘-Wno-parentheses-equality’
cc1: warning: unrecognized command line option ‘-Wno-constant-logical-operand’
make: *** [Makefile:248: linux_inet_diag.o] Error 1
```

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103492

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:113845] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


Thanks for the report, fixed in https://github.com/ioquatix/raindrops/commit/94dbdd94977d895f98c084d0ca31c2b9cf0d25d3

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103493

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:114014] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[kamil-gwozdz via ruby-core]

Issue #19057 has been updated by kamil-gwozdz (Kamil Gwóźdź).


byroot (Jean Boussier) wrote in #note-17:
> @ioquatix I'm not sure which change exactly is the cause, but it appears that the recent `rb_io_t` changes broke `raindrops`


I've noticed the same thing while trying to install `kgio` with the latest ruby HEAD.

{{collapse
```
make DESTDIR\= sitearchdir\=./.gem.20230623-102030-g2vcyh sitelibdir\=./.gem.20230623-102030-g2vcyh
compiling accept.c
In file included from accept.c:7:
my_fileno.h:9:7: warning: "HAVE_RB_IO_T" is not defined, evaluates to 0 [-Wundef]
    9 | #if ! HAVE_RB_IO_T
      |       ^~~~~~~~~~~~
my_fileno.h:16:8: warning: "HAVE_RB_IO_T" is not defined, evaluates to 0 [-Wundef]
   16 | #  if !HAVE_RB_IO_T || (RUBY_VERSION_MAJOR == 1 && RUBY_VERSION_MINOR == 8)
      |        ^~~~~~~~~~~~
my_fileno.h: In function ‘my_fileno’:
my_fileno.h:10:19: error: unknown type name ‘OpenFile’; did you mean ‘GetOpenFile’?
   10 | #  define rb_io_t OpenFile
      |                   ^~~~~~~~
my_fileno.h:25:2: note: in expansion of macro ‘rb_io_t’
   25 |  rb_io_t *fptr;
      |  ^~~~~~~
In file included from kgio.h:6,
                 from accept.c:4:
/workspaces/project/vendor/ruby/30c1fc4087744da4dcc1138e23cf43118e10f5af/include/ruby-3.3.0+0/ruby/io.h:395:55: warning: assignment to ‘int *’ from incompatible pointer type ‘struct rb_io *’ [-Wincompatible-pointer-types]
  395 | #define RB_IO_POINTER(obj,fp) rb_io_check_closed((fp) = RFILE(rb_io_taint_check(obj))->fptr)
      |                                                       ^
/workspaces/project/vendor/ruby/30c1fc4087744da4dcc1138e23cf43118e10f5af/include/ruby-3.3.0+0/ruby/io.h:401:21: note: in expansion of macro ‘RB_IO_POINTER’
  401 | #define GetOpenFile RB_IO_POINTER
      |                     ^~~~~~~~~~~~~
my_fileno.h:30:2: note: in expansion of macro ‘GetOpenFile’
   30 |  GetOpenFile(io, fptr);
      |  ^~~~~~~~~~~
/workspaces/project/vendor/ruby/30c1fc4087744da4dcc1138e23cf43118e10f5af/include/ruby-3.3.0+0/ruby/io.h:395:55: warning: passing argument 1 of ‘rb_io_check_closed’ from incompatible pointer type [-Wincompatible-pointer-types]
  395 | #define RB_IO_POINTER(obj,fp) rb_io_check_closed((fp) = RFILE(rb_io_taint_check(obj))->fptr)
      |                                                  ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                       |
      |                                                       int *
/workspaces/project/vendor/ruby/30c1fc4087744da4dcc1138e23cf43118e10f5af/include/ruby-3.3.0+0/ruby/io.h:401:21: note: in expansion of macro ‘RB_IO_POINTER’
  401 | #define GetOpenFile RB_IO_POINTER
      |                     ^~~~~~~~~~~~~
my_fileno.h:30:2: note: in expansion of macro ‘GetOpenFile’
   30 |  GetOpenFile(io, fptr);
      |  ^~~~~~~~~~~
/workspaces/project/vendor/ruby/30c1fc4087744da4dcc1138e23cf43118e10f5af/include/ruby-3.3.0+0/ruby/io.h:638:34: note: expected ‘rb_io_t *’ {aka ‘struct rb_io *’} but argument is of type ‘int *’
  638 | void rb_io_check_closed(rb_io_t *fptr);
      |                         ~~~~~~~~~^~~~
In file included from accept.c:7:
my_fileno.h:17:41: error: request for member ‘f’ in something not a structure or union
   17 | #    define FPTR_TO_FD(fptr) fileno(fptr->f)
      |                                         ^~
my_fileno.h:31:7: note: in expansion of macro ‘FPTR_TO_FD’
   31 |  fd = FPTR_TO_FD(fptr);
      |       ^~~~~~~~~~
accept.c: At top level:
cc1: note: unrecognized command-line option ‘-Wno-self-assign’ may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option ‘-Wno-parentheses-equality’ may have been intended to silence earlier diagnostics
cc1: note: unrecognized command-line option ‘-Wno-constant-logical-operand’ may have been intended to silence earlier diagnostics
make: *** [Makefile:248: accept.o] Error 1

make failed, exit code 2
```
}}



----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103675

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:114104] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[k0kubun (Takashi Kokubun) via ruby-core]

Issue #19057 has been updated by k0kubun (Takashi Kokubun).


> I've noticed the same thing while trying to install kgio with the latest ruby HEAD.

It looks like @byroot sent a patch for this at https://yhbt.net/kgio-public/B5843088-6EC1-4D1B-A45A-2699CA75DD7F@gmail.com/T/#u.

Both patches are still not released. Unless @normalperson cuts a timely release for kgio and raindrops, we will not be able to use Unicorn on Ruby 3.3 because of this Feature.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103779

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:114106] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[nobu (Nobuyoshi Nakada) via ruby-core]

Issue #19057 has been updated by nobu (Nobuyoshi Nakada).


k0kubun (Takashi Kokubun) wrote in #note-20:
> It looks like @byroot sent a patch for this at https://yhbt.net/kgio-public/B5843088-6EC1-4D1B-A45A-2699CA75DD7F@gmail.com/T/#u.

`rb_convert_type(io, T_FILE, "IO", "to_io")` can be `rb_io_get_io(io)`.


----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103781

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:114298] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[k0kubun (Takashi Kokubun) via ruby-core]

Issue #19057 has been updated by k0kubun (Takashi Kokubun).


`cool.io` is another gem broken by this feature. FYI, I filed a pull request here https://github.com/tarcieri/cool.io/pull/79.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-103996

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Target version: 3.3
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:114491] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


- `cool.io` has since been fixed and released.
- `unicorn` has merged a fix but it is not released: https://yhbt.net/unicorn.git/63c85c4282d15e22bd32a905883d2d0e149619d1/s/
- `kgio` has a patch submitted.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-104276

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Target version: 3.3
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:114521] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[naruse (Yui NARUSE) via ruby-core]

Issue #19057 has been updated by naruse (Yui NARUSE).


I'll release Ruby 3.3.0 preview 2 soon.
I'm concerning that those three projects don't support the preview yet.
A preview release is to allow people to test their applications, but without support of those foundational projects it's hard to achieve the goal.
If those projects are not fixed before the preview release, I'll revert changes related to Feature #19057 for preview 2.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-104317

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Target version: 3.3
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:114522] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


@naruse Here is the compatibility fix that will allow `kgio`, `unicorn` and so on to compile with no changes: https://github.com/ruby/ruby/pull/8286

Please feel free to merge that before doing the preview 2 release, if @normalperson has not released updated gems.

However, bear in mind this delays the inevitable - I'd still like to propose we remove this completely in 3.4 - 3.3 has deprecations, and 3.4 is removed. Does that sound okay to you?

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-104318

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Target version: 3.3
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:114627] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[byroot (Jean Boussier) via ruby-core]

Issue #19057 has been updated by byroot (Jean Boussier).


Eric just said he'll cut new `unicorn` and `kgio` releases: https://yhbt.net/kgio.git/dbf5290cf9f89174f6b35a597af9a4226633d79b/s/

> Will push out a release once docs are updated to more strongly
discourage the use of kgio and unicorn.



----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-104455

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Target version: 3.3
----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:116197] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


Related PR: https://github.com/ruby/ruby/pull/9530

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-106213

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:116198] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


Update from 5 months ago:

- unicorn has merged a fix but it is not released: https://yhbt.net/unicorn.git/63c85c4282d15e22bd32a905883d2d0e149619d1/s/
- kgio has merged a fix but it is not released: https://yhbt.net/kgio.git/dbf5290cf9f89174f6b35a597af9a4226633d79b/s/
- raindrops merged a fix but it is not released: https://yhbt.net/raindrops.git/b3212417cc3e7cc44aa9e1ffe89b0d62ef3fdab5/s/

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-106214

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:116211] Re: [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eric Wong via ruby-core]

"ioquatix (Samuel Williams) via ruby-core" <ruby-core@ml.ruby-lang.org> wrote:
> - unicorn has merged a fix but it is not released: https://yhbt.net/unicorn.git/63c85c4282d15e22bd32a905883d2d0e149619d1/s/
> - kgio has merged a fix but it is not released: https://yhbt.net/kgio.git/dbf5290cf9f89174f6b35a597af9a4226633d79b/s/
> - raindrops merged a fix but it is not released: https://yhbt.net/raindrops.git/b3212417cc3e7cc44aa9e1ffe89b0d62ef3fdab5/s/

Still working on writing release notes and docs.
<snip> sharing common struct fields example (more below)

> 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.

It's safe.  Normal (unpacked) C structs have a well-defined and
stable ABI.  Ruby has been relying on it with RVALUE union and
RBasic->flags for decades.  FFI users all rely on this C ABI
stability for calling C code w/o a C compiler.

gcc and clang have a __transparent_union__ attribute designed
to make migrations like this easy and painless for old code.

rb_io_descriptor also introduces extra function call overhead.
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:116213] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


Are you saying that in general:

```
struct A {
  int x;
  float y;
  char z;
  struct S s;
};
```

and

```
struct B {
  int x;
  float y;
};
```

That reinterpreting a pointer to A as a pointer to B, and accessing field `B::y` is never undefined? And that generalises? The only rule I knew was the first field, if a struct, is guaranteed to be at offset=0. Are you able to link me to the standard?

While I understand what you are saying may be true, I was not able to find confirmation of it in the standard.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-106233

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:116377] Re: [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eric Wong via ruby-core]

"ioquatix (Samuel Williams) via ruby-core" <ruby-core@ml.ruby-lang.org> wrote:
> Are you saying that in general:
> 
> ```
> struct A {
>   int x;
>   float y;
>   char z;
>   struct S s;
> };
> ```
> 
> and
> 
> ```
> struct B {
>   int x;
>   float y;
> };
> ```

> That reinterpreting a pointer to A as a pointer to B, and accessing
> field `B::y` is never undefined? And that generalises? The only rule I
> knew was the first field, if a struct, is guaranteed to be at offset=0.
> Are you able to link me to the standard?

(oops, battery died last week and I forgot about Ruby entirely :x)

As long as it's the same HW arch, yes.  AFAIK C itself doesn't say anything
about it, it's up to every ABI.  https://refspecs.linuxfoundation.org/elf/
has a bunch of ABIs.  abi386-4.pdf (via pdftotext) states:

  Each member is assigned to the lowest available offset with the appropriate
  alignment. This may require internal padding, depending on the previous
  member.

> While I understand what you are saying may be true, I was not able to
> find confirmation of it in the standard.

It's a lot of arch-specific ABIs to read through and I haven't
read every one; but keep in mind FFI/Rust interop would not work
at all without a predictable C ABI.  I do similar things via
Perl syscall wrapper and pack (same as Ruby Array#pack) on
various architectures the GCC Compiler Farm (cfarm) provides
with no problems.

You can always add STATIC_ASSERT on the offsets to ensure new
platforms are behaving.  I don't expect Ruby will be able to run
at all if there's a hypothetical platform with unpredictable offsets.

All this only applies to C and normal C types (not C++)
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:116379] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eregon (Benoit Daloze) via ruby-core]

Issue #19057 has been updated by Eregon (Benoit Daloze).


Agreed, from the hardware POV it has to store the struct in a reasonable/consistent manner in memory.
I would be more worried about the C compiler, if the C compiler figures out a pointer is casted to "unrelated" struct types it might consider that as undefined behavior and do anything.
But I suspect that's explicitly not undefined behavior, because C has no notion of struct types being related or not, and there is likely tons of software using structs "inheriting/extending" another smaller struct.
It might not explicitly be allowed either because I guess the fully correct way to do this is using a union of both structs, and not casting to a specific struct directly then.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-106396

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:116389] Re: [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eric Wong via ruby-core]

"Eregon (Benoit Daloze) via ruby-core" <ruby-core@ml.ruby-lang.org> wrote:
> I would be more worried about the C compiler, if the C
> compiler figures out a pointer is casted to "unrelated" struct
> types it might consider that as undefined behavior and do
> anything.

It's not undefined, every platform defines its own C ABI.
Any toolchain which ignores that ABI cannot interoperate at all
(which is fine for cases where you don't need interop)

Consider 1) how many languages/runtimes call C libraries.
Now consider 2) how many languages/libraries get called by C.

1 is common, 2 is rare and needs explicit instructions in
the non-C language (e.g. extern "C" in C++ headers).

gcc and clang generate objects from C which can safely be linked
by the others' linker.  This isn't true for C++ at all.

FFI is completely dependent on a stable ABI.

> But I suspect that's explicitly not undefined behavior,
> because C has no notion of struct types being related or not,
> and there is likely tons of software using structs
> "inheriting/extending" another smaller struct.
> 
> It might not explicitly be allowed either because I guess the
> fully correct way to do this is using a union of both structs,
> and not casting to a specific struct directly then.

FFI and Rust would not be able to use C libraries if the
C toolchain were told to violate its platform ABI.

Consider "struct sockaddr *" use in C standard library:

Functions like accept, connect, bind, getpeername, getsockname,
etc. all take a "struct sockaddr *" arg, but callers are expected
to allocate one of sockaddr_{in,in6,un,storage,...} to call them.

No unions are forced on a user for those sockaddr functions.
(I end up defining unions myself in what little C code I maintain,
but that's my choice for maintainability).

Same for POSIX fcntl locks, various ioctls, getsockopt/setsockopt,
or any other function which takes multiple struct types.

You can use FFI or syscall+pack/unpack on all of those
functions.

To make things more explicit, gcc and clang has
transparent_union which is intended to help document and
preserve ABI compatibility:
https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Common-Type-Attributes.html#index-transparent_005funion-type-attribute
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117204] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[mame (Yusuke Endoh) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117206] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117207] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[byroot (Jean Boussier) via ruby-core]

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/6215d4cad7a964bf4b8bdef48edadf334eae73ee



----------------------------------------
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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117208] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117220] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[mame (Yusuke Endoh) via ruby-core]

Issue #19057 has been updated by mame (Yusuke Endoh).


Why don't you reconsider the "nested public interface" approach?

From the reaction to this ticket, it is clear that forcing the "hide all the details" approach could destroy the Ruby ecosystem. And there is no need to force it because you have a more moderate alternative approach.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107306

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117224] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[matz (Yukihiro Matsumoto) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117226] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117228] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


Here is the revert PR: https://github.com/ruby/ruby/pull/10283

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107315

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117229] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eregon (Benoit Daloze) via ruby-core]

Issue #19057 has been updated by Eregon (Benoit Daloze).


> From the reaction to this ticket, it is clear that forcing the "hide all the details" approach could destroy the Ruby ecosystem.

"destroy the Ruby ecosystem" seems an exaggeration if it's just `unicorn` not working, because there was no release in 2+ years.
Are we going to never remove an API because one gem seems not maintained anymore and relies on it?

It might even be useful if people notice unicorn is no longer maintained actively, e.g. if security issues are found there might not be a release fixing them either.
Note that I would be very happy to be proven wrong about unicorn being no longer maintained.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107316

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117230] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


As an alternative course of action, I've released the latest head of unicorn as `unicorn-maintained` gem. You can use this instead of `unicorn` and it will work on Ruby head.

https://github.com/unicorn-ruby/unicorn

Anyone who wants to help contribute/maintain it, I am happy to add you to the organisation. It also includes `raindrops-maintained` as this has not been released.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107317

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117262] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[mame (Yusuke Endoh) via ruby-core]

Issue #19057 has been updated by mame (Yusuke Endoh).


ioquatix (Samuel Williams) wrote in #note-43:
> Here is the revert PR: https://github.com/ruby/ruby/pull/10283

Thanks!

Just FYI, I think the precedence in Ruby's decision is basically:

* beauty of the language (conciseness and intuitiveness for users) >= compatibility > runtime performance and efficiency >>> beauty of the implementation (simplicity for the core developers).

Compared to the past, compatibility has become much more of a priority, and performance has become somewhat more important. The order is sometimes reversed in rare cases, but the beauty of the implementation has always been the lowest priority.

The "beauty of the implementation", such as hiding the rb_io_t implementation, can beat "compatibility" only in limited and special cases, e.g., only when there is no other way at all, or when the practical impact is very small (no affected gem is identified, or it is only an issue with minor gems). In this case, unicorn, which is undeniably an important gem and still part of the Ruby ecosystem, is affected and there are known implementation workarounds. In that case, there is no reason not to choose the workaround.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107355

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117267] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eregon (Benoit Daloze) via ruby-core]

Issue #19057 has been updated by Eregon (Benoit Daloze).


Re beauty of the implementation this change is more than that, we can see in https://github.com/ruby/ruby/pull/10283/files that `rb_io_t` exposes way too much internals, which in turns prevents changing/evolving these internals without affecting extensions, can easily break ABI unintentionally, can have negative performance impact, etc.
It's also rather brittle with the internal duplicate definition of rb_io_t, and could easily cause segfaults if the structs don't match (e.g. when working on this code and then wasting a bunch of time due to that, or maybe subtle enough that the CI would not catch and it would only be found later).

I hope we can still land this change, if it needs to wait 3.5 or so then that sounds a good compromise.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107392

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117289] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


At @mame's request, I've reverted this change. The reversion itself was straightforward, yet it does not mitigate the underlying concerns that have come to light through this work. My attempt to enhance Ruby's `IO` implementation has highlighted a vulnerability within our ecosystem: the infrequency of updates and the ambiguity surrounding future releases from the maintainers of essential gems, such as `unicorn`.

This predicament places the Ruby community in a precarious position, not just in terms of adopting improvements but more importantly, in our ability to respond to security vulnerabilities. The reliance on these gems by a significant segment of our community intensifies the potential impact of any unresolved issues.

@mame if you truly believe a malfunctioning `unicorn` can "destroy the Ruby ecosystem", then I fear we have bigger problems that need to be discussed - it would only take a security issue or two to put us all companies that depend on `unicorn` in a very difficult position. Because I care about the future of Ruby, I made the decision to make a community fork of `unicorn` as I don't see any other realistic option.



----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107429

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117290] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[k0kubun (Takashi Kokubun) via ruby-core]

Issue #19057 has been updated by k0kubun (Takashi Kokubun).


> I don't see any other realistic option. I wish there was another way.

FWIW, @byroot built a fork https://github.com/shopify/pitchfork, which has a reforking feature Unicorn doesn't have, and Shopify uses that for our largest application in production.

> The reliance on these gems by a significant segment of our community increases the potential impact of any unresolved issues.

I'd say it's unrealistic to expect all existing users to switch to something else before the Ruby 3.4 release. For example, Sprockets v3 is much more popular than v4 ([source](https://bestgems.org/gems/sprockets)) while it was released 6 years ago and is way past its EOL. Since that version depends on the legacy interface of ERB (which I maintain), the gem's legacy interface (deprecated 6 years ago) is very much stuck, like `rb_io_t`. But I'd rather not break existing Rails applications unless it's absolutely necessary. I don't think it's more important to hide implementation details than to keep existing applications running.

Since the author himself [strongly discourages the use of Unicorn](https://yhbt.net/kgio-public/20230905092243.M939104@dcvr/), users should, however, _eventually_ stop using Unicorn. For those who are looking for Unicorn alternatives, you may use Pitchfork, or Puma with 1 thread per process (for Unicorn compatibility).

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107430

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117298] Re: [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eric Wong via ruby-core]

ioquatix (Samuel Williams) wrote:
> - Companies could contact Eric and offer incentives for him to make a release.

That's not possible, https://yhbt.net/unicorn/ISSUES states:

  The author of unicorn must never be allowed to profit off the damage it's
  done to the entire Ruby world.

I'm 100% banned for life from ever profitting off anything related to unicorn.

"mame (Yusuke Endoh) via ruby-core" <ruby-core@ml.ruby-lang.org> wrote:
> Why don't you reconsider the "nested public interface" approach?

Samuel: please do this.  Ruby even has (Linux||ccan) `container_of'
macro as another option:

	struct rb_io_private {
		struct rb_io { // public ABI
			int fd;
			// any other public fields in used in real-world
		} io_pub;
		// private stuff here
		// private fields can go above `io_pub', too
	};

Then only expose the `io_pub' field to public structs and access
rb_io_private via ccan_container_of.

But the previously discussed ways are valid C since every known
platform has a stable ABI (otherwise FFI would never work)

I expect there are other gems and private extensions affected by
this C API change (if they survived the 1.8 -> 1.9 changes).

> From the reaction to this ticket, it is clear that forcing the
> "hide all the details" approach could destroy the Ruby
> ecosystem. And there is no need to force it because you have a
> more moderate alternative approach.

Too bad that's already happened over the decades I've been
around Ruby.  Ruby lost numerous users due to a neverending
stream of incompatibilities introduced every year.  The only way
I can maintain the legacy Ruby code I still have is to rewrite
tests in a different language (e.g. Perl or POSIX sh (NOT bash))

I'm completely burned out with having to constantly deal with a
never ending stream of incompatibilities over the past ~20
years.  This mentality has propagated to the entire ecosystem;
e.g. Rack::Chunked was deprecated and my proposed patches sent
to rack-devel@googlegroups.com to maintain compatibility were
completely ignored in Sep 2022.

frozen_string_literal will be another major pain point, and
the nagging from chilled strings won't do much to make things
better (I thought that was decided against a decade ago).


Finally, MFA on Rubygems is a misguided corporate attempt at
security.  I'm an amateur volunteer refuse to be held
responsible for the security of multi-billion dollar
corporations.  I've never claimed any professional or academic
qualifications.  Nobody knows me, nobody ever will; I only show
you code and that's all anybody should need for security.


I'll probably end up self-hosting my own gems and only put
future releases on a self-hosted server.  Of course, I claim no
qualifications in security or systems administration.


Users are welcome to fork (and pitchfork exists) if they'd
rather live under the boot of corporate rule and Terms of
Service that can change at any time.  I'm not going to put
myself in a position where I can't contribute to code I still
depend on.  I'm already effectively banned from 99.9% of
projects due to draconian corporate terms of service and
high HW requirements.
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117300] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[ioquatix (Samuel Williams) via ruby-core]

Issue #19057 has been updated by ioquatix (Samuel Williams).


> Why don't you reconsider the "nested public interface" approach?

My assessment of this approach is that it would require a rewrite of all internal code that accesses `rb_io_t` internals. Rewriting code isn't a problem, but it takes a lot of time and effort that I don't have in abundance right now. Additionally, it's still only a partial solution. One of the issues is accessing the file descriptor directly and the handling of `IO#close` from a different thread/fiber. Using the file descriptor directly can be problematic.

> Ruby lost numerous users due to a never-ending stream of incompatibilities introduced every year.

Compatibility and progress are always something to balance out. I think on the whole, Ruby does a pretty decent job at it. Lack of forward progress also causes users to look elsewhere as the language stagnates. So this isn't just. a matter of "preserve compatibility at all costs", as those costs will be the same. The hard part is finding the middle ground of compatibility and progress. Both compatibility at any cost, and progress at any cost, are naive statements.


----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107442

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117301] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[mame (Yusuke Endoh) via ruby-core]

Issue #19057 has been updated by mame (Yusuke Endoh).


Eric, thanks for your comment. I understand your concern about Ruby's incompatibility.

Sometimes it is inevitable to introduce incompatibilities to improve a language specification (you may not agree with me here), but I believe that we shoud make reasonable efforts to avoid other incompatibilities.

I am not familiar with the situation of Rack, but I have heard a rumor that the incompatibilities in Rack 3 are so large that the migration has not progressed. That's unfortunate.
Also, `frozen_string_literal` change was unfortunately approved, which I personally still believe that is the wrong direction. It is really a shame to have an inconvenient and inconsistent language specification for a performance reason.

I am also sorry about RubyGems MFA. I think the policy like Eric's is not very uncommon in the OSS world. I can do nothing because I am not involved in the RubyGems operation, but I hope the RubyGems team will give some consideration to a policy like yours.

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107443

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117310] [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[byroot (Jean Boussier) via ruby-core]

Issue #19057 has been updated by byroot (Jean Boussier).


> the nagging from chilled strings 

Eric, I understand that it won't make your annoyance go away, but just in case you didn't know, if you wish to get rid of these warnings easily, you can add `# frozen_string_literal: false` at the top of all Unicorn's source files, like so:

https://github.com/unicorn-ruby/unicorn/commit/23da1fc46a18ed0330081b55892b34476e7910e5.patch

----------------------------------------
Feature #19057: Hide implementation of `rb_io_t`.
https://bugs.ruby-lang.org/issues/19057#change-107452

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:117361] Re: [Ruby master Feature#19057] Hide implementation of `rb_io_t`.

[Eric Wong via ruby-core]

"ioquatix (Samuel Williams) via ruby-core" <ruby-core@ml.ruby-lang.org> wrote:
> Issue #19057 has been updated by ioquatix (Samuel Williams).
> 
> 
> > Why don't you reconsider the "nested public interface" approach?
> 
> My assessment of this approach is that it would require a rewrite of all internal code that accesses `rb_io_t` internals. Rewriting code isn't a problem, but it takes a lot of time and effort that I don't have in abundance right now. Additionally, it's still only a partial solution. One of the issues is accessing the file descriptor directly and the handling of `IO#close` from a different thread/fiber. Using the file descriptor directly can be problematic.

Again, why not the container_of solution I proposed?

It shouldn't be that much for you since the relevant core
internals could be divorced from extensions and significantly
less work for stressed out and overworked downstream maintainers.


Cross-thread IO#close or close(2) is a PITA to deal with
portably if another thread is using that FD in any way.
This applies to pure C projects, too, not just Ruby ones.
Either:

1) ensure only one thread operates on a given FD and closes it;
   this is typical for stream FDs.

2) get all threads to abandon the FD before closing it.
   I do this for thread-shared packetized FDs which are
   analogous to Queue (O_DIRECT pipe, SOCK_SEQPACKET,
   listen sockets, kqueue/epoll FDs, etc...).

Atomics + out-of-band (pthread_kill) signaling work for both
cases.  I've also used sentinel values in-band for packetized
FDs (analogous to pushing `:stop' messages into a thread-shared
Queue, like forcing a connect() in one thread to get blocking
accept() to wake up; or adding an (eventfd||pipe) into a
(kqueue||epoll) from another thread (it's 100% safe to
EPOLL_CTL_ADD||EV_ADD from different threads and I've been
abusing this safety for well over a decade)

> > Ruby lost numerous users due to a never-ending stream of incompatibilities introduced every year.
> 
> Compatibility and progress are always something to balance out. I think on the whole, Ruby does a pretty decent job at it. Lack of forward progress also causes users to look elsewhere as the language stagnates. So this isn't just. a matter of "preserve compatibility at all costs", as those costs will be the same. The hard part is finding the middle ground of compatibility and progress. Both compatibility at any cost, and progress at any cost, are naive statements.

git and Linux kernel do a pretty good job at maintaining
compatibility while adding new features.  OTOH some of the
incompatibility proposals for Perl 5 have made me look into
writing more C...
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/