Fix FileDescriptor#pos return Int64 on armv6
#10845
Conversation
straight-shoota
added
kind:bug
straight-shoota
force-pushed
the
fix/file-pos-armv6
branch
from
bd5f584 to
1509694
Compare
| @@ -80,7 +80,7 @@ module Crystal::System::FileDescriptor | |||
| end | |||
|
|
|||
| private def system_pos | |||
| pos = LibC.lseek(fd, 0, IO::Seek::Current) | |||
| pos = LibC.lseek(fd, 0, IO::Seek::Current).to_i64! | |||
There was a problem hiding this comment.
Why does it need to_i64! if to_i64 already introduces no additional runtime checks for types <= 64
There was a problem hiding this comment.
I accociate the exclamation mark method when I know the cast is safe and doesn't need any check. But we can use #to_i64 as well. I'm not sure I care either way.
There was a problem hiding this comment.
We happen to live in a world where we can say that this will never be Int128. But why rely on that instead of just writing the expression that's always correct? I think those ! methods need a high bar for acceptance.
There was a problem hiding this comment.
Ha, on the other hand maybe even if it was Int128 we'd prefer to assume that there are never such huge files. Whatever :D
| @@ -13,7 +13,7 @@ class File::PReader < IO | |||
| @pos = 0 | |||
There was a problem hiding this comment.
Not directly related, but pos really should be Int64
There was a problem hiding this comment.
I added it anyways. It's in a similar mood.
straight-shoota
changed the title
FileDescriptor#pos return Int64 on armv6
Since #10580, the return type of
IO::FileDescriptor#posis restricted toInt64. This means it's actually broken on armv6.Similarly,
IO::FileDescriptor#preadwas missing a type restriction and returned different types depending on the target platform. This is getting unified.This resolves the initial reports of #10645, but there may be more (#10645 (comment)).