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

CLines - Deriving from Array(String) Breaks Unrelated Code in my Project using Array #30

Open
stellarpower opened this issue Apr 21, 2023 · 3 comments

Comments

@stellarpower
Copy link

stellarpower commented Apr 21, 2023

Deriving from Standard library types apparently is deprecated (I don't understand the reasons myself) - and the following line defining CLines as deriving from Array(String) means that then Array(T) in the standard library is being deduced to Array(T)+ in external code as part of the same project. This playground link summarises the core of the issue.

This has caused my code to break and not to build when trying to include code requiring Crysterm along with code requiring a different shard. Code internal to the other shard breaks with a confusing message:

Error: expected argument #2 to 'Hash(Char, Array(UInt8).class)#[]=' to be Array(UInt8).class, not Array(UInt8).class

A trivial MRE is available here - note that I don't need to use anything from either library; simply requiring both within the same project causes an error to come from within the osc-crystal library.

I'm coming to Crystal from Ruby originally - and then most of my background is in other languages where deriving from a class in no way is able to affect that class, so personally afraid I'm not understanding what is going on here or why deriving from a type should cause issues with other code that uses this type (in my mind that's surely the basic way inheritance is meant to work). So apologies, but I don't have much I can usefully add in the description here.

But I've been informed that deriving from types in the standard library is probably not good, and that therefore it's worth opening this as an issue here. I'll also be opening a bug for the Crystal compiler itself, as IMO the error message definitely needs some clarification - Array(UInt8) != Array(UInt8) leaves one head-scratching for quite a while.

Potential Solutions:

  • Change CLines to use composition, although I imagine this would involve a significant amount of work in adding boilerplate to proxy functions to what is currently the base class
  • ... (not sure on what else myself, sorry!)

Thanks

@docelic
Copy link
Collaborator

docelic commented Apr 21, 2023

Heya, thanks for the report, the issue looks somewhat unfortunate :)
I will look into it in the next few days. Will update as soon as I have something new.

The idea of delegating to a proxy object might work.
I think it'd even be relatively easy to do since Crystal supports these two things:

https://crystal-lang.org/api/1.8.1/Object.html#forward_missing_to%28delegate%29-macro
https://crystal-lang.org/api/1.8.1/Object.html#delegate%28%2Amethods%2Ctoobject%29-macro

@stellarpower
Copy link
Author

No worries! I have had to implement what I needed using bare curses, so no rush form my part.

I assumed macros might come to the rescue here, that's quite a nice feature to have in the stdlib. The fact that this happened in the first place still gives me the heebie-jeebies - I'm assuming there's a reason for this and it won't change, but I guess there's a chance the bug I opened generates further discussion, as breaking an unrelated library from creating a new type is like, quite a bad thing to happen as I see it.

@docelic
Copy link
Collaborator

docelic commented Apr 21, 2023

Since the Crystal team recognizes the report as a bug, let's also see if they will express any plans in crystal-lang/crystal#13377 to fix this maybe.

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

No branches or pull requests

2 participants