Merge pull request #71 from upstat-io/dev

fix(codegen): fix Option match tag inversion in decision tree (C4)

Author: elucidsoft

.github/workflows/ci.yml

@@ -191,12 +191,12 @@ jobs:
         if: steps.rust-tests.outcome == 'failure' || steps.ori-tests.outcome == 'failure' || steps.rt-tests.outcome == 'failure'
         run: exit 1
 
-  # Cross-platform smoke test - just verify it compiles and basic tests pass
+  # Cross-platform — full test suite including AOT
   cross-platform:
     name: ${{ matrix.os }}
     needs: [format, clippy]  # Don't waste CI time if basics fail
     runs-on: ${{ matrix.os }}
-    timeout-minutes: 12
+    timeout-minutes: 10
     strategy:
       fail-fast: false
       matrix:
@@ -235,44 +235,12 @@ jobs:
       - name: Build (all crates incl. LLVM)
         run: cargo build --workspace
 
-      - name: Rust unit tests (excl. LLVM AOT — tested on Linux)
-        run: cargo test --workspace --exclude ori_llvm
+      - name: Rust unit tests
+        run: cargo test --workspace
 
       - name: Ori language tests
         run: cargo run -p oric --bin ori -- test tests/
 
-      - name: AOT smoke test (compile + link + run)
-        shell: bash
-        run: |
-          SMOKE_DIR="${RUNNER_TEMP}/aot_smoke"
-          mkdir -p "$SMOKE_DIR"
-
-          # Write a minimal Ori program that exercises string + print (touches ori_rt)
-          cat > "$SMOKE_DIR/smoke.ori" << 'ORIEOF'
-          @main () -> void = print(msg: "hello from AOT");
-          ORIEOF
-
-          EXE_SUFFIX=""
-          if [[ "$RUNNER_OS" == "Windows" ]]; then
-            EXE_SUFFIX=".exe"
-          fi
-
-          BUILD_ARGS=(build "$SMOKE_DIR/smoke.ori" -o "$SMOKE_DIR/smoke${EXE_SUFFIX}")
-
-          # macOS: put Homebrew LLVM on PATH so lld is available
-          if [[ "$RUNNER_OS" == "macOS" ]]; then
-            LLVM_BIN="$(brew --prefix llvm@21)/bin"
-            if [[ -d "$LLVM_BIN" ]]; then
-              export PATH="$LLVM_BIN:$PATH"
-              if command -v lld >/dev/null 2>&1; then
-                BUILD_ARGS+=(--linker=lld)
-              fi
-            fi
-          fi
-
-          cargo run -p oric --bin ori -- "${BUILD_ARGS[@]}"
-          "$SMOKE_DIR/smoke${EXE_SUFFIX}"
-
   # Single status check for branch protection
   ci-success:
     name: CI Success

compiler/ori_arc/src/decision_tree/emit.rs

@@ -297,8 +297,8 @@ fn lookup_variant_field_type(
             }
         }
         Tag::Option => {
-            // Some (index 1) has one field (the inner type), None (index 0) has none.
-            if variant_index == 1 && field_index == 0 {
+            // Some (index 0) has one field (the inner type), None (index 1) has none.
+            if variant_index == 0 && field_index == 0 {
                 return pool.option_inner(resolved);
             }
         }

compiler/ori_arc/src/decision_tree/flatten.rs

@@ -231,7 +231,7 @@ impl<'a> FlattenCtx<'a> {
     ///
     /// Handles three cases:
     /// - `Tag::Enum`: looks up variant by name in the enum definition
-    /// - `Tag::Option`: `None` = 0, `Some` = 1
+    /// - `Tag::Option`: `Some` = 0, `None` = 1
     /// - `Tag::Result`: `Ok` = 0, `Err` = 1
     #[expect(
         clippy::cast_possible_truncation,
@@ -251,8 +251,8 @@ impl<'a> FlattenCtx<'a> {
                 }
             }
             Tag::Option => {
-                // Convention: None = 0, Some = 1 (matches evaluator's Value::Some/None)
-                return u32::from(self.interner.lookup(variant_name) == "Some");
+                // Convention: Some = 0, None = 1 (matches construction in lower_some/lower_none)
+                return u32::from(self.interner.lookup(variant_name) == "None");
             }
             Tag::Result => {
                 // Convention: Ok = 0, Err = 1 (matches evaluator's Value::Ok/Err)
@@ -273,7 +273,7 @@ impl<'a> FlattenCtx<'a> {
     /// Get the field types for a specific variant of an enum, Option, or Result.
     ///
     /// - `Tag::Enum`: returns field types from the enum definition
-    /// - `Tag::Option`: `Some` (index 1) has one field (the inner type), `None` (index 0) has none
+    /// - `Tag::Option`: `Some` (index 0) has one field (the inner type), `None` (index 1) has none
     /// - `Tag::Result`: `Ok` (index 0) has one field (ok type), `Err` (index 1) has one field (err type)
     fn resolve_variant_field_types(
         &self,

compiler/ori_eval/src/exec/decision_tree/mod.rs

@@ -254,6 +254,12 @@ fn resolve_bindings(
 // Test matching
 
 /// Check if a runtime value matches a decision tree edge's test value.
+#[expect(
+    clippy::match_same_arms,
+    reason = "Option and Result are independent type families with independently-defined \
+              tag conventions. Merging Some|Ok and None|Err arms hides the per-family \
+              semantics and caused the original C4 tag inversion bug."
+)]
 fn test_matches(
     value: &Value,
     _test_kind: TestKind,
@@ -272,8 +278,12 @@ fn test_matches(
                 variant_name: vn, ..
             } => *vn == *variant_name,
             // Some/None/Ok/Err are represented as special Value variants.
-            Value::Some(_) | Value::Err(_) => *variant_index == 1,
-            Value::None | Value::Ok(_) => *variant_index == 0,
+            // Convention: Some = 0, None = 1 (matches lower_some/lower_none)
+            Value::Some(_) => *variant_index == 0,
+            Value::None => *variant_index == 1,
+            // Convention: Ok = 0, Err = 1 (matches lower_ok/lower_err)
+            Value::Ok(_) => *variant_index == 0,
+            Value::Err(_) => *variant_index == 1,
             _ => false,
         },
 

compiler/ori_eval/src/exec/decision_tree/tests.rs

@@ -189,7 +189,7 @@ fn switch_option_tag() {
         edges: vec![
             (
                 TestValue::Tag {
-                    variant_index: 1,
+                    variant_index: 0,
                     variant_name: interner.intern("Some"),
                 },
                 DecisionTree::Leaf {
@@ -199,7 +199,7 @@ fn switch_option_tag() {
             ),
             (
                 TestValue::Tag {
-                    variant_index: 0,
+                    variant_index: 1,
                     variant_name: interner.intern("None"),
                 },
                 DecisionTree::Leaf {

compiler/ori_llvm/tests/aot/spec.rs

@@ -765,6 +765,52 @@ fn test_aot_result_err_check() {
     );
 }
 
+/// C4 regression: Option match tag inversion — switch labels must match construction tags.
+/// Construction: Some=tag 0, None=tag 1. Match must use the same mapping.
+#[test]
+fn test_aot_option_match_tag_correctness() {
+    assert_aot_success(
+        r#"
+@unwrap_or (opt: Option<int>, default: int) -> int =
+    match opt { Some(v) -> v, None -> default }
+
+@main () -> int = {
+    let some_val = unwrap_or(opt: Some(42), default: 0);
+    let none_val = unwrap_or(opt: None, default: 99);
+    // some_val should be 42 (not 0), none_val should be 99 (not garbage)
+    if some_val == 42 then {
+        if none_val == 99 then 0 else 1
+    } else 1
+}
+"#,
+        "option_match_tag_correctness",
+    );
+}
+
+/// C4 regression: match on Option inside if/else producing Option values.
+#[test]
+fn test_aot_option_match_with_construction() {
+    assert_aot_success(
+        r#"
+@safe_div (a: int, b: int) -> Option<int> =
+    if b == 0 then None else Some(a / b);
+
+@unwrap_or (opt: Option<int>, default: int) -> int =
+    match opt { Some(v) -> v, None -> default }
+
+@main () -> int = {
+    let a = unwrap_or(opt: safe_div(a: 100, b: 5), default: 0);
+    let b = unwrap_or(opt: safe_div(a: 100, b: 0), default: 5);
+    // a should be 20, b should be 5
+    if a == 20 then {
+        if b == 5 then 0 else 1
+    } else 1
+}
+"#,
+        "option_match_with_construction",
+    );
+}
+
 #[test]
 fn test_aot_option_some_unwrap() {
     assert_aot_success(

plans/llvm-codegen-fixes/section-01-critical-correctness.md

@@ -1,7 +1,7 @@
 ---
 section: "01"
 title: "Critical Correctness"
-status: not-started
+status: in-progress
 goal: "All 12 code journeys produce correct results in both eval and AOT paths"
 inspired_by:
   - "Rust rustc_codegen_llvm/mir/operand.rs — OperandValue variant handling"
@@ -10,7 +10,7 @@ depends_on: []
 sections:
   - id: "01.1"
     title: "Fix C4 — Option match tag inversion"
-    status: not-started
+    status: complete
   - id: "01.2"
     title: "Fix C3 — Payload sum type $eq codegen"
     status: not-started
@@ -62,14 +62,14 @@ switch i64 %proj.0, label %bb4 [
 
 **Root cause hypothesis:** The match codegen for monomorphized built-in generic types uses a different variant ordering than construction. The `?` operator avoids this because it uses a direct `icmp eq` rather than a `switch`.
 
-- [ ] Reproduce with Journey 12's code — AOT returns 144 instead of 33
-- [ ] Write targeted test: `Option<int>` construction + match, compare eval vs AOT
-- [ ] Write test: user-defined `type MyOpt<T> = MySome(v: T) | MyNone` — should pass (confirms isolation)
-- [ ] Trace the match codegen path for `Option<int>` — find where variant tag → switch label mapping diverges
-- [ ] Find the divergence between user-defined sum type match (correct) and built-in generic match (inverted)
-- [ ] Fix the root cause — ensure match switch labels match construction tags for all sum types
-- [ ] Verify `?` operator still works (it uses a different code path)
-- [ ] Journey 12 AOT returns 33
+- [x] Reproduce with Journey 12's code — AOT returns 144 instead of 33
+- [x] Write targeted test: `Option<int>` construction + match, compare eval vs AOT
+- [x] Write test: user-defined `type MyOpt<T> = MySome(v: T) | MyNone` — should pass (confirms isolation)
+- [x] Trace the match codegen path for `Option<int>` — find where variant tag → switch label mapping diverges
+- [x] Find the divergence between user-defined sum type match (correct) and built-in generic match (inverted)
+- [x] Fix the root cause — ensure match switch labels match construction tags for all sum types
+- [x] Verify `?` operator still works (it uses a different code path)
+- [x] Journey 12 AOT returns 33
 
 ---
 
@@ -175,9 +175,9 @@ PANIC: ValueId 4294967295 out of bounds
 - [ ] Journey 5 (closures): AOT returns 27, matching eval
 - [ ] Journey 10: list indexing test case returns correct element value
 - [ ] Journey 11 (derived Eq): AOT returns 33, matching eval
-- [ ] Journey 12 (Option match): AOT returns 33, matching eval
+- [x] Journey 12 (Option match): AOT returns 33, matching eval
 - [ ] All 12 journeys: `./scripts/dual-exec-verify.sh` — 0 mismatches
-- [ ] No new regressions: `./test-all.sh` green
-- [ ] `./clippy-all.sh` green
+- [x] No new regressions: `./test-all.sh` green
+- [x] `./clippy-all.sh` green
 
 **Exit Criteria:** All 4 critical bugs fixed. `./scripts/dual-exec-verify.sh` runs all 12 journey programs through both eval and AOT, producing 0 mismatches. No test regressions.