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

No type error when deleting from an array an invalid type #988

Open
refi64 opened this issue Jul 16, 2015 · 8 comments
Open

No type error when deleting from an array an invalid type #988

refi64 opened this issue Jul 16, 2015 · 8 comments

Comments

@refi64
Copy link
Contributor

refi64 commented Jul 16, 2015

Example:

ar = [] of Int32
ar.delete "abc"

This has caused bugs for me before.

@refi64
Copy link
Contributor Author

refi64 commented Jul 16, 2015

Real-life example:

Consider these two functions:

      # Mark a register as no longer used.
      def freereg(reg)
        @usedregs.delete reg
      end

      # Free a register if it's a register item.
      def ofree(maybereg)
        self.freereg maybereg.reg if maybereg.is_a? Parser::RegItem
      end

I had originally written the second-to-last line as:

self.freereg maybereg if maybereg.is_a? Parser::RegItem

@usedregs is supposed to be of the type Register, which is completely independent from Parser::RegItem. Because I was passing the wrong type to freereg, the register was never freed. This caused that to lurk as a bug until my compiler crashed because it ran out of registers.

@refi64
Copy link
Contributor Author

refi64 commented Jul 16, 2015

Possible fix: change line 418 of array.cr to:

def delete(obj : T)

@asterite
Copy link
Member

This is interesting :-)

In Java for example removing an element is not restricted to a type. This is because you delete elements by equality. So, this works in Ruby (and in Crystal):

a = [1, 2, 3]
a.delete 1.0
a #=> [2, 3]

So I'm not sure what we should do here. This is an excellent opportunity to discuss it :-)

@refi64
Copy link
Contributor Author

refi64 commented Jul 16, 2015

@asterite Well, Ruby is duck-typed, and Java is pretty stupid. ;)

I'm just saying this because I don't think anyone has ever done that intentionally, as it doesn't really serve a purpose. What would the point of it be? a could never hold floats, anyway.

Just my three cents. :)

@vendethiel
Copy link
Contributor

I think I side with @kirbyfan64 here, for the sole purpose of catching type errors earlier

@asterite
Copy link
Member

I think it makes sense to put a type restriction there. The same happens with Hash#[], which doesn't have a restriction but maybe it should.

@straight-shoota
Copy link
Member

For hash there's #8893

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

No branches or pull requests

8 participants