Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return type of unbuffered_read/unbuffered_write #14377

Open
straight-shoota opened this issue Mar 20, 2024 · 2 comments
Open

Return type of unbuffered_read/unbuffered_write #14377

straight-shoota opened this issue Mar 20, 2024 · 2 comments

Comments

@straight-shoota
Copy link
Member

unbuffered_read and unbuffered_write are defined as abstract defs in IO::Buffered. They both have type restrictions on the slice parameter, but there's no restriction on the return type.

unbuffered_read is supposed to return the number of bytes written and IO::Buffered#write (which uses it), casts that number to Int32. So anything else than Int32 doesn't make sense for unbuffered_read and just causes unnecessary type variation (ref #10645).

There are currently two implementations returning a different type:

  • File::PReader#unbuffered_read : Int64
  • Crystal::System::FileDescriptor#unbuffered_read : UInt32 on Windows

Casting those values to Int32 could potentially raise, but in practice the value is limited to the size of the slice parameter, which is also an Int32.

For unbuffered_write, the return type is Nil because implementations are expected to write the slice fully.

The other abstract unbuffered_* methods in IO::Buffered are also lacking a return type restriction.

All these methods should have a return type restriction to clearly state what implentation are supposed to do.
Unfortunately, this would be a breaking change because implementations that don't explicitly define the return type restriction would error, even when the return type matches.

But we can at least add the return type to the documentation and express that these are expected to be required at some point.
And of course, the implementations in stdlib itself should get return type restrictions in order to comply with a future restriction of the abstract signature.

Another step further could be to make the return type restrictions optionally available via a compiler flag. I wouldn't expect this to be used much, so it might not be very helpful unless we get some kind of compatibility profile to opt into (#11706).

@crysbot
Copy link

crysbot commented Mar 21, 2024

This issue has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/ensuring-smooth-language-changes-for-2-0/3588/19

@crysbot
Copy link

crysbot commented Mar 21, 2024

This issue has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/ensuring-smooth-language-changes-for-2-0/3588/21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants