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 deferrals #1402

Merged
merged 8 commits into from
Oct 29, 2024
Merged

Fix deferrals #1402

merged 8 commits into from
Oct 29, 2024

Conversation

7h3kk1d
Copy link
Contributor

@7h3kk1d 7h3kk1d commented Oct 1, 2024

Before
Screenshot from 2024-10-03 09-52-57
After
Screenshot from 2024-10-03 09-53-12

@7h3kk1d 7h3kk1d changed the base branch from dev to addl_tests2 October 1, 2024 17:26
Comment on lines 359 to 357

let remaining_arg_ty =
List.length(remaining_args) == 1
? snd(List.hd(remaining_args))
: Prod(List.map(snd, remaining_args)) |> Typ.temp;
DeferredAp(f'', args'')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems right to me. I don't think we always want the remaining function to be from a product.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code still works without this in my test though. So TODO

let. _ = otherwise(env, (d1, (d2, _)) => Ap(dir, d1, d2) |> rewrap)
and. d1' =
req_value(req(state, env), d1 => Ap1(dir, d1, d2) |> wrap_ctx, d1)
req_final(req(state, env), d1 => Ap1(dir, d1, d2) |> wrap_ctx, d1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't have a strong sense as to why this needs to be req_final but it remains indet otherwise.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Negabinary not sure I understand what req_value was doing here since we still need to handle casts? are casts considered values?

In any case, deferredap should be treated as if it were a lambda, so it should be a value.

Let's chat briefly about this with @7h3kk1d at the beginning of our meeting tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cyrus- Oh I see, yeah I guess if you still want to do the function cast rules when the function is indet then you do need that to be req_final

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got all of the cases I can think of working. I'm struggling to find a case that req_final catches that req_value doesn't though. There might be something wrong with Constructor

Base automatically changed from addl_tests2 to dev October 1, 2024 18:19
@7h3kk1d 7h3kk1d marked this pull request as ready for review October 4, 2024 20:57
@7h3kk1d 7h3kk1d requested review from cyrus- and Negabinary October 4, 2024 20:57
@cyrus- cyrus- merged commit 73a7598 into dev Oct 29, 2024
2 checks passed
@cyrus- cyrus- deleted the fix-deferrals branch October 29, 2024 18:57
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