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

Fix invalid returns in class getter's lazy evaluation blocks #15364

Merged

Conversation

ysbaddaden
Copy link
Contributor

Using return in a class getter will immediately return from the generated method, never setting the class variable to the returned value, which is kept nil so the initializer code is called repeatedly instead of being called once then cached.

Extracted from #15340

Using `return` in a class getter will immediately return from the
generated method, never setting the class variable to the returned
value, which is kept nil and so the initializer code is called
repeatedly instead of being called _once_ then cached.
@ysbaddaden ysbaddaden self-assigned this Jan 22, 2025
@RX14
Copy link
Contributor

RX14 commented Jan 22, 2025

I suspect this could be quite a common breaking change in practice, I'd rather not make this change if at all possible. It also adds another footgun for future implementations that isn't easily caught. Or does this still happen with the current class getter impl?

@ysbaddaden
Copy link
Contributor Author

@RX14 This patch only fixes a couple identified issues, whatever if we start capturing the block or not: the behavior was wrong.

@straight-shoota
Copy link
Member

Yeah this is not changing anything, it just fixes incorrect uses of return in a class getter block. They are already broken: return exits from the outer context and thus doesn't assign the ivar, which means the lazy initialization evaluates over and over again.
So you basically never want that.

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib kind:specs and removed kind:bug A bug in the code. Does not apply to documentation, specs, etc. labels Jan 23, 2025
@straight-shoota straight-shoota added this to the 1.16.0 milestone Jan 23, 2025
@straight-shoota straight-shoota changed the title Fix: invalid returns in a couple class getters Fix invalid returns in class getter's lazy evaluation blocks Jan 23, 2025
@straight-shoota straight-shoota merged commit bac59ec into crystal-lang:master Jan 24, 2025
70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants