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

Allow try without catch #4600

Merged
merged 6 commits into from
Jul 9, 2024
Merged

Allow try without catch #4600

merged 6 commits into from
Jul 9, 2024

Conversation

ggreif
Copy link
Contributor

@ggreif ggreif commented Jul 4, 2024

UPDATE: I reverted a lot of it, see commit 1a7b61d for an explanation.

see #4507

TODO:

  • check that no IR-scrutinising happens — see below
  • use catch_opt consistently
  • suspicious: IR-interpreter and await.ml didn't need changes? — see below

In the IR when none of the catch alternatives match, then an artificially added

@ [{ it = { pat = varP error; exp = varE f -*- varE error };

re-throws the Error. So df9389c shortcuts the creation of a silly do-nothing failure continuation when the list of alternatives is empty. It would just re-throw unconditionally anyway!

Optimisation opportunity: only emit this when all of the patterns are refutable! — Done: 2329069

@ggreif ggreif changed the title WIP: frontend Allow try without catch Jul 4, 2024
Copy link

github-actions bot commented Jul 4, 2024

Comparing from f511bb0 to aa4d7d8:
The produced WebAssembly code seems to be completely unchanged.

when there is only a default (catch-all) that would rethrow
anyway
ggreif added 2 commits July 9, 2024 12:12
it is not helpful:
- longer
- we may have several `catch`-es in the future
- inconsistent with IR (which is still a `list`)
- no advantages
@ggreif ggreif marked this pull request as ready for review July 9, 2024 10:28
@ggreif ggreif added the opportunity More optimisation opportunities inside label Jul 9, 2024
@ggreif ggreif merged commit 84aa3e2 into gabor/finally Jul 9, 2024
3 of 5 checks passed
ggreif added a commit that referenced this pull request Jul 9, 2024
@ggreif ggreif removed the opportunity More optimisation opportunities inside label Jul 9, 2024
mergify bot pushed a commit that referenced this pull request Jul 25, 2024
Introduces an optional `finally` clause to `try` expressions that can be used for disciplined (structured) resource management. The clause comprises a unit-valued expression (or block) that may not perform sends (or similar). It gets executed for all control paths that leave the `try`-expression, i.e. `return`, `break`, etc. code paths. For nested `try` expressions the cleanups are performed in innermost to outermost order.

For last-resort cleanups (i.e. code paths trapping after an `await`) the replica invokes the callback registered as `ic0.call_on_cleanup`, and this will result in the execution of (exclusively) the `finally` blocks in (dynamic) scope. This is a new mechanism in Motoko, which was not possible to achieve earlier, and puts certain resource deallocations (e.g. of acquired locks) under the programmer's control.

-----------------
TODO:
- [ ] [invoke cleanup from the interpreter](https://github.com/dfinity/motoko/pull/4507/files/51bb8a1933bbf2c509447032f1ba26b8f91fa4e0#r1658678631)
- [ ] make `catch` optional (non-trapping when missing) — #4600
  - well, this was not really needed as pattern-match failure would just re-`throw`, so we were fine. Optimised, though.
- [x] remaining FIXMEs (`arrange.ml`, etc.)
- [x] `Triv` in `try` (respect edges)
  - [x] fix fallout
- [x] forbid control-flow edges out of `finally` blocks
- [x] remove all aliasing from IR
  - [ ] use `meta` (would `assert` currently, thus causing minor code duplication) — could be follow-up PR
- [x] handle `Throw` and regular continuations in `await.ml` (instead of desugaring)
  - [x] adjust (AST, IR)-interpreter
  - ❗️this is necessary when there is no `catch`, as the exception will escape
- how are label continuations invoked in the backend? (I need to know for the _last-resort callback_)
  - have a new `context` key sort `Cleanup`, stack up continuations of finally blocks only
  - extend the `kr` in `await.ml` to `krc` (`CPSAwait`), pass `Cleanup` continuations as `c`
  - add new primitive to compiler to set the last-resort field (not needed)
  - in `async.ml` lower `CPSAwait` to that too
  - [x] adapt `check_ir.ml`, etc.
- [x] tests with `await*`
- [x] fix up the syntax part
- `do` ... `finally` (when there is no `catch` clause necessary)
  - ~means: `TryE` needs `cases list option`~
- type checking for the `finally` clause
  -  [x] unit result
  -  [x] trivial effect (actually Claudio made this capability-based)
  -  [ ] that the `try` block has `await` effect — nope!
- [x] other type than unit for `try`
- `OtherPrim "call_raw"` + tests — nope!
- `cps_asyncE` similarly to `CallPrim/3` — nope!
- highlighting of `finally`
  - [x] for `emacs`
  - [x] for VSCode → dfinity/vscode-motoko#288
  - [x] GitHub highlighter
- [x] Changelog (breaking change, new keyword)
- [x] Docs
- [ ] adapt best practices: https://internetcomputer.org/docs/current/developer-docs/security/security-best-practices/inter-canister-calls#recommendation
- [ ] adapt Motoko examples where locking happens
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.

1 participant