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

IO::Hexdump#read is returning (Int32 | Int64) instead of Int32 #10855

Closed
caspiano opened this issue Jun 28, 2021 · 4 comments · Fixed by #10856
Closed

IO::Hexdump#read is returning (Int32 | Int64) instead of Int32 #10855

caspiano opened this issue Jun 28, 2021 · 4 comments · Fixed by #10856
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:regression Something that used to correctly work but no longer works
Milestone

Comments

@caspiano
Copy link
Contributor

This stack trace is produced from some code tested against nightly.

Looks like the annotation here is incorrect, or an Int64 is slipping in somewhere.

@caspiano caspiano added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Jun 28, 2021
@caspiano
Copy link
Contributor Author

Here's another failing CI run with the same error, but different stack trace.

@straight-shoota straight-shoota added the kind:regression Something that used to correctly work but no longer works label Jun 28, 2021
@straight-shoota
Copy link
Member

This is caused by #10575 (or #10580 more specifically) which adds lots of useful type restrictions. This seems to be one of the few cases where this causes an issue. For the time being, we should simply revert all restrictions that cause any trouble. The hard part is identifying them.
Theoretically, we could just revert all those PRs and make sure to review them more carefully. But I think that would be too drastic. The error rate is really low.

I think I'll go through all the PRs again and try to identify any methods that might be problematic as well. The most likely trouble makers are probably integer type restrictions.

@straight-shoota
Copy link
Member

Actually, maybe we should just cast to Int32 instead. I mean, we really want unified return types for any IO#read implementation.

This can probably wait until after 1.1 and reverting this is cheap for a quick fix.

@caspiano
Copy link
Contributor Author

There’s no type restriction on IO#read (@io in the class is not a more specific subclass of IO), so I would wait for that instead of casting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:regression Something that used to correctly work but no longer works
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants