Skip to content

Add if (var x = expr) and map[k].field op= rhs#19

Open
congwang-mk wants to merge 7 commits intomainfrom
iflet-and-map-field-compound
Open

Add if (var x = expr) and map[k].field op= rhs#19
congwang-mk wants to merge 7 commits intomainfrom
iflet-and-map-field-compound

Conversation

@congwang-mk
Copy link
Copy Markdown
Contributor

Summary

Two new statement forms for ergonomic map access, plus the doc and test work pinning them down.

  • if (var x = expr) { ... } else { ... } — declaration-as-condition. Binds x only inside the truthy branch; the branch is taken iff expr produces a present value (map hit, non-null pointer return). Lowers to a single bpf_map_lookup_elem plus a null check; for struct-valued maps the body's field operations mutate the entry in place via the underlying lookup pointer (no manual write-back).
  • m[k].field op= rhs — compound assignment on a struct-field of a map value. Lowers to a presence-checked if (p) { p->field = p->field op rhs; } against the same single lookup. Absent entries are a no-op (no creation), distinguishing it from the existing scalar m[k] op= rhs which creates on first use.

Bundled deliverables:

  • examples/maps_demo.ks and examples/map_operations_demo.ks: rewrite the read-modify-write blocks to use the new idiom (drops 13 LoC, removes redundant write-backs).
  • README.md: replace the misleading lookup_or_create snippet with paired examples; add an "Ergonomic map idioms" bullet to the solution checklist.
  • SPEC.md: new §6.2.5 (compound assignment with map indexing — scalar and struct-field forms with full semantics: LHS shape, type rules, presence/creation semantics, lowering shape), new §7.5.1.2 (declaration-as-condition with single-evaluation, scoping, and lowering); assignment_statement and if_statement EBNF productions expanded from the prior shorthand into the seven forms the parser actually implements. Also fixes the §6.2.3 example, which used stats.field += 1 on a value-bound stats — a shape the parser has never accepted.
  • tests/test_iflet.ml (new): 10 tests across parse, type-check, and codegen-string layers — covers struct/scalar bindings, else, else if (var ...) chains, binding-leak rejections (then-branch only), and the in-place-mutation codegen shape.
  • tests/test_compound_index_assignment.ml: extend the Phase 2 test with a codegen-string check pinning the synthetic-pointer-binding path that the codegen-fix in this PR repaired (verified by temporarily reverting the fix — test flips red on "synthetic binding does NOT use deref-load init").

84 alcotest suites pass (was 83); the new test_iflet suite is the +1.

Test plan

  • eval \$(opam env) && dune build && dune runtest --force — 84 suites green
  • cd /tmp && cp <example>.ks . && kernelscript compile <example>.ks -o out && cd out && make — example end-to-end build for maps_demo.ks (clean) and a minimal m[k].field += 1 source (clean)
  • Skim the eBPF C output for a Phase 2 source — confirms bpf_map_lookup_elem happens once, presence is checked against NULL, and the mutation is ptr->field = ...
  • Spot-check generated C for a struct-map IfLet — confirms the dead value-typed local declaration plus the in-place-mutation through the lookup pointer (this is the existing user-written-IfLet shape, locked in by tests/test_iflet.ml::codegen_struct_value_binding_shape)

🤖 Generated with Claude Code

Signed-off-by: Cong Wang <cwang@multikernel.io>
Signed-off-by: Cong Wang <cwang@multikernel.io>
Signed-off-by: Cong Wang <cwang@multikernel.io>
… and SPEC

Signed-off-by: Cong Wang <cwang@multikernel.io>
Signed-off-by: Cong Wang <cwang@multikernel.io>
Signed-off-by: Cong Wang <cwang@multikernel.io>
…resence checks

Signed-off-by: Cong Wang <cwang@multikernel.io>
@congwang-mk congwang-mk force-pushed the iflet-and-map-field-compound branch from 00ad003 to d5f25c1 Compare May 5, 2026 21:37
@congwang-mk congwang-mk closed this May 5, 2026
@congwang-mk congwang-mk reopened this May 5, 2026
@SiyuanSun0736
Copy link
Copy Markdown
Contributor

1. Scoping Issue: Branch-Local Binding and Shadowing

I have identified a high-risk scoping issue regarding the if (var x = expr) syntax. Currently, while the frontend treats this as a branch-local binding, the lowering process expands it into a plain declaration followed by an if (x != null) check.

This approach appears to lose the then-branch-only scoping semantics, which is particularly problematic when x shadows an existing variable from an outer scope. Since the backends collect variables at function scope, the inner binding may collapse onto the outer name instead of being treated as a distinct variable.

Proposed Solutions:

  • Lower the binding to a fresh, synthesized name to ensure uniqueness.
  • Update the IR to carry explicit scope information that the backend must respect.
  • Action: Add a regression test specifically for shadowing an outer binding within this construct.

2. Semantic Mismatch: Truthiness and Evaluation Rules

There appears to be a significant semantic mismatch in how if (var x = expr) is handled across the toolchain. Currently, the syntax accepts arbitrary expressions, but the interpreter evaluates them using general truthy/falsy rules, while the lowering phase compiles the check as x != null.

This creates a discrepancy for values like 0 or false, where the interpreter's evaluation and the generated eBPF code will disagree on the outcome.

Proposed Solutions:

  • Restrict Scope: Limit this form to "presence-producing" expressions only, such as map lookups and nullable pointers.
  • Unify Rules: Ensure the evaluator, type checker, and lowering logic share a single, consistent rule for truthiness.
  • Explicit Presence: Consider introducing a first-class Option-like abstraction (with Some/None) to handle presence explicitly, rather than overloading null for all cases.

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.

2 participants