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

Make exit() a builtin #1416

Closed
wants to merge 1 commit into from
Closed

Make exit() a builtin #1416

wants to merge 1 commit into from

Conversation

plajjan
Copy link
Contributor

@plajjan plajjan commented Aug 6, 2023

Instead of env.exit() we now have exit(cap: WorldCap, exit_code: int) as a builtin function. The primary benefit is that functions are executed synchronously unlike actor methods which are async. Thus, env.exit() typically required using await async env.exit(0) whereas as a function it is called as a synchronous procedure, so exit(env.cap, 0) is enough.

Having to add await async meant users could do the wrong thing. With exit() as a function, there is no ambiguity. The user cannot do anything wrong! Also, this is shorter and more to the point. Less confusing to new usres. The only weird thing now is the capability argument...

I think we should have a new ExitCap capability, but I want to keep down the amount of clutter right now, in particular for something as important as 'exit'. Beginners will see it early and it's scary when it look all too scary. Accepting WorldCap means we can do exit(env.cap, 0) otherwise it would probably be like exit(ExitCap(env.cap), 0) and it's just extra bits complicating things. I think we should have ExitCap but as exit() is so common that it should be acceptable to use WorldCap directly. For the few programs where one actually wants to delegate a restricted ExitCap capability, that should be possible to by having an ExitCap but it would require exit() to accept a union, either an ExitCap or WorldCap. We don't have unions yet, so it will have to wait.

Fixes #1028

Instead of env.exit() we now have exit(cap: WorldCap, exit_code: int) as
a builtin function. The primary benefit is that functions are executed
synchronously unlike actor methods which are async. Thus, env.exit()
typically required using `await async env.exit(0)` whereas as a function
it is called as a synchronous procedure, so `exit(env.cap, 0)` is
enough.

Having to add `await async` meant users could do the wrong thing. With
exit() as a function, there is no ambiguity. The user cannot do anything
wrong! Also, this is shorter and more to the point. Less confusing to
new usres. The only weird thing now is the capability argument...

I think we should have a new ExitCap capability, but I want to keep down
the amount of clutter right now, in particular for something as
important as 'exit'. Beginners will see it early and it's scary when it
look all too scary. Accepting WorldCap means we can do `exit(env.cap,
0)` otherwise it would probably be like `exit(ExitCap(env.cap), 0)` and
it's just extra bits complicating things. I think we should have ExitCap
but as exit() is so common that it should be acceptable to use WorldCap
directly. For the few programs where one actually wants to delegate a
restricted ExitCap capability, that should be possible to by having an
ExitCap but it would require exit() to accept a union, either an ExitCap
or WorldCap. We don't have unions yet, so it will have to wait.
@plajjan
Copy link
Contributor Author

plajjan commented Aug 6, 2023

@nordlander I'd like you to have a look at this. We have talked about turning exit into a builtin function instead of a method on env, but it was a long time ago so want to reconfirm this is what we want.

I think it's a good move, in summary:

  • only behavior is the expected behavior
    • env.exit() can be called async (default) or forced sync (adds line noise)
  • less noise
    • we trade await async for feeding a capability as arg to exit()
      • but it's mandatory, so no way to mess it up

With env.exit() the naïve way of calling it (without await async) is "the wrong thing", i.e. not what the user wants / thinks env.exit does. That exit is asynchronously is probably a surprise to most users! With exit as a function, it is called synchronously, i.e. what most people would expect.

It's important to consider this now since I think exit is very common and part of the shape and feel of the language!

@plajjan
Copy link
Contributor Author

plajjan commented Aug 15, 2023

@nordlander and I have spoken about it, rather in-depth this time. To avoid sounding like a indecisive flip-flopping flip-flop going back and forth between ideas, let me first explain the first idea of having a synchronous exit(). This was a bit of a knee jerk reaction. I had written some program and was very surprised that after I ran env.exit() I would see output from my program after env.exit() was called. This is natural since env is an actor and so env.exit() is an actor method call and when not assigning the return value, actor method calls are done asynchronuosly. I happened to have a structure in my program that allowed some print statement to run after env.exit(). It should probably have been structured differently.

It is a reasonable idea that env.exit() would more or less exit the program immediately. However, we would like to offer some mechanism for hooking into the shutdown of a program, like installing a callback with env. If we immediately exit then such callbacks would not be called.

So if the goal indeed is to exit immediately, then converting exit to a builtin function would be the right solution with the argument laid out in this PR description & commit. However, the realization is that this probably should not be the goal. Rather, shutdown of an actor world simply looks different and it is better we educate our users about the expectation and how to structure programs accordingly... thus closing this PR without merging.

@plajjan plajjan closed this Aug 15, 2023
@plajjan plajjan mentioned this pull request Aug 15, 2023
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.

Add exit as builtin
1 participant