Skip to content

Conversation

phischu
Copy link
Collaborator

@phischu phischu commented Jun 12, 2025

No description provided.

@phischu phischu requested review from b-studios and removed request for b-studios June 13, 2025 08:33
@phischu phischu changed the title Promises in terms of Futures Promises in terms of Channels Jun 13, 2025
@phischu phischu force-pushed the concurrency branch 2 times, most recently from 94f661d to 6a24a05 Compare June 13, 2025 08:52
@b-studios b-studios requested a review from Copilot June 16, 2025 14:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the old promise-based implementation in C and Effekt with a channel-based API, renames the wait function to sleep in the time library, and updates example imports accordingly.

  • Switch C-side Promise to Channel with states EMPTY, SENDED, WAITED
  • Rename extern async def wait to sleep in io/time.effekt
  • Add new io/channel and io/promise Effekt modules and update examples

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
libraries/llvm/io.c Replaced Promise struct and functions with Channel
libraries/common/io/time.effekt Renamed wait to sleep in time API
libraries/common/io/promise.effekt Added new Effekt promise module
libraries/common/io/channel.effekt Added new Effekt channel module
libraries/common/io.effekt Removed old promise code, slimmed imports
examples/stdlib/io/time.effekt Updated example to use sleep
examples/stdlib/io/promise.effekt Added import io/promise
examples/stdlib/io/filesystem/... Added import io/promise
examples/stdlib/acme.effekt Added import io/channel & io/promise
examples/benchmarks/.../interleave_promises.effekt Added import io/promise
Comments suppressed due to low confidence (1)

libraries/llvm/io.c:301

  • The state name 'SENDED' is grammatically incorrect; consider renaming it to 'SENT' to improve clarity.
typedef enum { EMPTY, SENDED, WAITED } channel_state_t;

Comment on lines 339 to 350
void c_channel_send(struct Pos channel, struct Pos value) {
Channel* f = (Channel*)channel.obj;
switch (f->state) {
case EMPTY: {
f->state = SENDED;
f->payload.value = value;
erasePositive(channel);
break;
case RESOLVED:
erasePositive(promise);
}
case SENDED: {
erasePositive(channel);
erasePositive(value);
eraseStack(stack);
fprintf(stderr, "ERROR: Promise already resolved\n");
// TODO more graceful panic
fprintf(stderr, "ERROR: Channel already sended\n");
exit(1);
break;
}
case WAITED: {
Stack stack = f->payload.stack;
f->state = EMPTY;
erasePositive(channel);
resume_Pos(stack, value);
break;
}
}
// TODO stack overflow?
// We need to erase the promise now, since we consume it.
erasePositive(promise);
}

void c_promise_await(struct Pos promise, Stack stack) {
Promise* p = (Promise*)promise.obj;

Stack head;
Listeners* tail;
Listeners* node;
struct Pos value;

switch (p->state) {
case UNRESOLVED:
head = p->payload.listeners.head;
tail = p->payload.listeners.tail;
if (head != NULL) {
node = (Listeners*)malloc(sizeof(Listeners));
node->head = head;
node->tail = tail;
p->payload.listeners.head = stack;
p->payload.listeners.tail = node;
} else {
p->payload.listeners.head = stack;
};
void c_channel_wait(struct Pos channel, Stack stack) {
Channel* f = (Channel*)channel.obj;
switch (f->state) {
case EMPTY: {
f->state = WAITED;
f->payload.stack = stack;
erasePositive(channel);
break;
case RESOLVED:
value = p->payload.value;
sharePositive(value);
}
case SENDED: {
struct Pos value = f->payload.value;
f->state = EMPTY;
erasePositive(channel);
resume_Pos(stack, value);
break;
};
// TODO hmm, stack overflow?
erasePositive(promise);
}

struct Pos c_promise_make() {
Promise* promise = (Promise*)malloc(sizeof(Promise));

promise->rc = 0;
promise->eraser = c_promise_erase_listeners;
promise->state = UNRESOLVED;
promise->payload.listeners.head = NULL;
promise->payload.listeners.tail = NULL;

return (struct Pos) { .tag = 0, .obj = promise, };
}
case WAITED: {
erasePositive(channel);
eraseStack(stack);
// TODO more graceful panic
fprintf(stderr, "ERROR: Channel already waited\n");
exit(1);
break;
}
}
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

There is a TODO for a more graceful panic; consider implementing structured error handling rather than calling exit directly.

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is C and this is an assertion violation.

}
case SENDED: {
struct Pos value = f->payload.value;
f->state = EMPTY;
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

In the SENDED branch of c_channel_wait, erasing the channel after setting its state to EMPTY prevents payload.value from being freed, leading to a memory leak. Consider explicitly erasing the value or adjusting the state before erasing.

Suggested change
f->state = EMPTY;
f->state = EMPTY;
erasePositive(value);

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nono, the value is moved!

@phischu phischu changed the title Promises in terms of Channels Promises in terms of Signals Jul 29, 2025
@phischu phischu requested review from marvinborner and removed request for marvinborner July 30, 2025 11:22
@phischu phischu requested a review from b-studios August 11, 2025 14:19
@phischu phischu changed the title Promises in terms of Signals Concurrency primitives Aug 16, 2025
@phischu phischu force-pushed the concurrency branch 3 times, most recently from b383f2a to 6cb85be Compare August 25, 2025 07:05
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.

2 participants