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

typechecker: Allow field references in methods #1381

Merged
merged 1 commit into from
Feb 20, 2023

Conversation

fahrradflucht
Copy link
Contributor

Allow creating local references to struct fields in methods.

It turned out the way we were checking scope lifetimes was a bit backwards. We use this method every to guard against the inverse (the child scope outliving the parent) but the method wasn't implemented that way. To be honest, I still don't understand a 100% why some tests were passing in the old implementation.

I also had to invert the exit condition to account for the fact that comptime scope parent chains are disjunct from runtime scope parent chains, but at least we now know that the json parser sample is testing for that case 😉.

Fixing this also surfaced that the stores_arguments attribute test wasn't actually testing the feature but was passing due to the bug. I had to adjust it to actually test the failure case for it to continue to pass.

Progress on #1379.

@BuggieBot
Copy link
Member

Hello!

One or more of the commit messages in this PR do not match the Jakt code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

@fahrradflucht fahrradflucht changed the title selfhost: Allow field references in methods. typecheck: Allow field references in methods. Feb 19, 2023
@fahrradflucht fahrradflucht changed the title typecheck: Allow field references in methods. typechecker: Allow field references in methods. Feb 19, 2023
@fahrradflucht fahrradflucht changed the title typechecker: Allow field references in methods. typechecker: Allow field references in methods Feb 19, 2023
Allow creating local references to struct fields in methods.

It turned out the way we were checking scope lifetimes was a bit
backwards. We use this method every to guard against the inverse (the
child scope outliving the parent) but the method wasn't implemented that
way. To be honest I still don't understand a 100% why some tests were
passing in the old implementation.

We I also had to invert the exit condition to account for the fact that
comptime scope parent chains are disjunct from runtime scope parent
chains, but at least we now know that the json parser sample is testing
for that case 😉.

Fixing this also surfaced that the `stores_arguments` attribute test
wasn't actually testing the feature but was passing due to the bug. I
had to adjust it to actually test the failure case for it to continue to
pass.

Progress on SerenityOS#1379.
@alimpfard alimpfard merged commit 85b87d3 into SerenityOS:main Feb 20, 2023
@fahrradflucht fahrradflucht deleted the fix-lifetime-checks branch February 20, 2023 18:48
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

Successfully merging this pull request may close these issues.

3 participants