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

Change value type to struct #13015

Open
straight-shoota opened this issue Jan 27, 2023 · 1 comment
Open

Change value type to struct #13015

straight-shoota opened this issue Jan 27, 2023 · 1 comment

Comments

@straight-shoota
Copy link
Member

straight-shoota commented Jan 27, 2023

In #8226 it was agreed that Process::Status is a value type and should be implemented as a struct, not a class.
This PR was never merged, though.

Maybe there are other types that fit that profile as well. It's important to note that not any immutable type should automatically be a struct. Process::Status is just a wrapper around a 32-bit integer, which makes a lot of sense (cf #8226 (comment)).

OAuth::RequestToken might be considered for a struct as well, but I think it's less obvious.

Changing a public stdlib type from class to struct would obviously be a breaking change. The question is whether it has any relevant impact. I think it probably won't be a big problem as it's only gonna have an effect for reopening the type. Reopening stdlib types is not particularly encouraged, so this might be an acceptable change.

In any case, this should be considered for Crystal 2.0 where such a breaking change is definitely possible. Maybe we can also fit it into a minor release.

Related: #13014

@asterite
Copy link
Member

For Crystal 2.0, I think one idea could be to have everything be a class. If you want something to have struct semantics you can use an annotation, like @[Struct]. That way when you reopen a type, it's always using class and there's less chance for a breaking change.

Of course there's still module, but changing things from class to module is less common.

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

No branches or pull requests

2 participants