Use ADT instead of sentinel values for weave control flow #140

New issue

Closed

opened 2025-12-30 15:24:50 +00:00 by navicore · 1 comment

navicore commented

2025-12-30 15:24:50 +00:00

(Migrated from github.com)

Background

PR #138 uses sentinel values (i64::MIN and i64::MIN + 1) to signal weave completion and cancellation through channels.

Current State

In crates/runtime/src/weave.rs:

const DONE_SENTINEL: i64 = i64::MIN;
const CANCEL_SENTINEL: i64 = i64::MIN + 1;

These are sent as Value::Int(DONE_SENTINEL) through channels and detected via pattern matching.

Problem

While unlikely, a user could legitimately yield i64::MIN or i64::MIN + 1, causing:

Premature completion detection
Unexpected cancellation behavior

This is documented as a limitation, but it's a design smell.

Proposed Solution

Replace sentinel values with an internal enum:

enum WeaveMessage {
    Value(Value),      // Normal yield/resume value
    Done,              // Weave completed naturally
    Cancel,            // Cancellation requested
}

Channel type changes from mpmc::channel::<Value>() to mpmc::channel::<WeaveMessage>().

Benefits

No collision with user values possible
Clearer code intent
Type-safe control flow
Removes documented limitation about i64::MIN

Trade-offs

Slightly more allocation (enum wrapper)
Minor refactoring across weave.rs

Acceptance Criteria

Remove DONE_SENTINEL and CANCEL_SENTINEL constants
Implement WeaveMessage enum
Update channel types and send/recv patterns
Remove limitation from documentation
All tests pass
Add test case that yields i64::MIN successfully

PR #138 (weave implementation)
Issue #131 (original generator request)

## Background PR #138 uses sentinel values (`i64::MIN` and `i64::MIN + 1`) to signal weave completion and cancellation through channels. ## Current State In `crates/runtime/src/weave.rs`: ```rust const DONE_SENTINEL: i64 = i64::MIN; const CANCEL_SENTINEL: i64 = i64::MIN + 1; ``` These are sent as `Value::Int(DONE_SENTINEL)` through channels and detected via pattern matching. ## Problem While unlikely, a user could legitimately yield `i64::MIN` or `i64::MIN + 1`, causing: - Premature completion detection - Unexpected cancellation behavior This is documented as a limitation, but it's a design smell. ## Proposed Solution Replace sentinel values with an internal enum: ```rust enum WeaveMessage { Value(Value), // Normal yield/resume value Done, // Weave completed naturally Cancel, // Cancellation requested } ``` Channel type changes from `mpmc::channel::<Value>()` to `mpmc::channel::<WeaveMessage>()`. ## Benefits - No collision with user values possible - Clearer code intent - Type-safe control flow - Removes documented limitation about `i64::MIN` ## Trade-offs - Slightly more allocation (enum wrapper) - Minor refactoring across weave.rs ## Acceptance Criteria - [ ] Remove `DONE_SENTINEL` and `CANCEL_SENTINEL` constants - [ ] Implement `WeaveMessage` enum - [ ] Update channel types and send/recv patterns - [ ] Remove limitation from documentation - [ ] All tests pass - [ ] Add test case that yields `i64::MIN` successfully ## Related - PR #138 (weave implementation) - Issue #131 (original generator request)

navicore commented

2025-12-30 16:23:47 +00:00

(Migrated from github.com)

https://github.com/navicore/patch-seq/pull/143