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

Range#size raises compile time error #13121

Closed
straight-shoota opened this issue Feb 27, 2023 · 3 comments · Fixed by #13278
Closed

Range#size raises compile time error #13121

straight-shoota opened this issue Feb 27, 2023 · 3 comments · Fixed by #13278
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection

Comments

@straight-shoota
Copy link
Member

straight-shoota commented Feb 27, 2023

Range#size raises a compile time error when either the begin or end type of the range are Nil. This method overrides Enumerable#size.

Raising at compile time effectively makes this method undefined for Range(T, Nil) and Range(Nil, T) and subsequently for Enumerable(T) and Enumerable(Nil), respectively. This breaks the expectation that a subtype inherits all methods of a parent type. When a range type with Nil parameter is instantiated anywhere in a program, it makes it impossible to call Enumerable#size because then not all inheriting types implement that method.

0.. 
[1, 2, 3].as(Enumerable(Int32)).size # Error: Can't calculate size of an open range

There is no way to work around that, except changing the argument type of Range from Nil (it can be nilable, which raises at runtime when the value is nil).

To fix this, Range#size must not raise at compile time.
The existing runtime error for nil values should be sufficient.

This compile time error was introduced in #8829.
The issue was first reported in Sija/debug.cr#7

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection labels Feb 27, 2023
@Sija
Copy link
Contributor

Sija commented Feb 27, 2023

Btw, several other Range methods are doing the same thing, see:

If we're going to proceed with removing the compile checks from #size, it makes sense to remove them from the other methods as well.

@devnote-dev
Copy link
Contributor

Isn't there a way to have this raise exclusively for Range? Having this checked at compile time makes it easier to catch accidental footguns and you are guaranteed to not run into this further down development, whereas raising at runtime does the opposite.

@straight-shoota
Copy link
Member Author

No that's not possible. Range#size is an implementation of Enumerable#size and will always be considered when a call to that method is made.

Considering that this issue hasn't been noticed in 3 years, I figure the practical relevance of footgun prevention is negligible.

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. topic:stdlib:collection
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants