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

Remove incorrect type restrictions on index methods with offset #10972

Conversation

straight-shoota
Copy link
Member

#10583 introduced return type restrictions to index methods that are not correct, because the return type depends on the type of the offset parameter.

Resolves #10969

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:text kind:regression Something that used to correctly work but no longer works labels Jul 20, 2021
@mattrberry
Copy link
Contributor

Just ran into this yesterday and started making changes for a PR. You beat me to it 😉 I wonder how many more regressions like this there are

Copy link
Member

@sdogruyol sdogruyol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @straight-shoota 🙏

@straight-shoota straight-shoota added this to the 1.1.1 milestone Jul 20, 2021
…nt32?` return type

`Slice(UInt8)#index(UInt8, Int)` delegates to `Slice#fast_index` which has a
return type of `Int32` but the compiler would not be able to figure that out
and merge in the type of `offset` into the union.
Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@straight-shoota straight-shoota merged commit d56ebc7 into crystal-lang:master Jul 22, 2021
@straight-shoota straight-shoota deleted the fix/string-index-return-type branch July 22, 2021 08:44
syeopite added a commit to iv-org/invidious that referenced this pull request Aug 12, 2021
This reverts commit f2b69fd.

Crystal 1.1.0 is not supported by Invidious due to upstream issue. See
crystal-lang/crystal#10972
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 topic:stdlib:text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type restriction of String#rindex
5 participants