Skip to content

Handle struct field write through array index in emit#72

Open
angelica-moreira wants to merge 3 commits into
mainfrom
fix/emit-struct-array-field-write
Open

Handle struct field write through array index in emit#72
angelica-moreira wants to merge 3 commits into
mainfrom
fix/emit-struct-array-field-write

Conversation

@angelica-moreira
Copy link
Copy Markdown
Member

@angelica-moreira angelica-moreira commented Mar 5, 2026

Problem

arr[idx].field = val produces admit() in the F*/Pulse output.

In StmtT::Assign, the LHS Member(Index(arr, idx), field) falls
through to emit_lvalue. Since ExprT::Index returns ExprKind::RValue
(correct — Pulse's arr.(idx) reads without !), emit_lvalue gets an
RValue and emits TODO/admit().

Fix

Extract emit_assign() helper from emit_stmt() so the Assign arm stays inside the annotated() wrapper. Pattern-match Member(Index(arr, idx), field) in the new helper, emitting a call to update_arr_with:
pts[i].x = val;

becomes:

let __val = (!var_val); update_arr_with (!var_pts) (!var_i) (fun __cur -> { __cur with struct_point__x = __val });

update_arr_with is a new generic helper added to Pulse.Lib.C.Array.fsti that reads, applies a pure function, and writes back. Its postcondition guarantees the element at the index equals f applied to the old element, which lets F* discharge _ensures specs like pts[i].x == val.

Spec expressions (_requires, _ensures, _invariant) now emit pure F* syntax for array indexing , Seq.index (value_of arr) (SizeT.v idx) instead of the Pulse-only syntax I previous used arr.(idx).

  • Scope limitation :
    • only handles one level of arr[i].field. Deeper patterns like arr[i].field.subfield or ptr->arr[i].field would need further work.

@angelica-moreira angelica-moreira requested a review from gebner March 5, 2026 00:04
Comment thread src/pass/emit.rs Outdated
struct_name.val.clone(),
field.val.clone(),
));
return arr_doc
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The early return skips the annotated wrapper, we should probably split this function in two (or make annotated take a closure).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok!

@gebner
Copy link
Copy Markdown
Contributor

gebner commented Mar 5, 2026

(correct — Pulse's arr.(idx) reads without !),

Yeah, it would be a lot cleaner if we could use array_at (the indexing operation which returns a reference), but automating the separation logic conditions seems far off at this point. So we'll need special cases like this in the foreseeable future, this approach is good.

Right now the code duplicates both the arr and idx expressions, which it shouldn't (if they have side effects). We should probably generate a call to an auxiliary function like update_arr_with arr idx (fun arr_val field -> { arr_val with field }) rhs.

Please also add a test case.

@angelica-moreira angelica-moreira force-pushed the fix/emit-struct-array-field-write branch 3 times, most recently from 07bfb1d to 1a885a6 Compare March 5, 2026 04:34
Handle arr[i].field = val by emitting:
  let __arr = arr;
  let __idx = idx;
  let __cur = __arr.(__idx);
  let __val = rhs;
  __arr.(__idx) <- { __cur with field = __val };

This uses Pulse-native let-statement bindings to:
- Avoid duplicating arr/idx expressions (side-effect safety).
- Read the current element into a pure variable before the
  record update, which Pulse's type checker requires.

Addresses review feedback:
- Extract emit_assign() helper so the Assign arm no longer
  bypasses the annotated() wrapper in emit_stmt.
- Use let-bindings for arr, idx, current element, and rhs
  to avoid duplicating expressions that may have side effects.
- Add test/struct_array_write.c exercising the pattern.

Verified: the generated Pulse code (with SizeT index) passes
F* verification with all conditions discharged successfully.

Scope limitation: Only handles one level of arr[i].field.
Deeper patterns like arr[i].field.subfield or ptr->arr[i].field
would need further work.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@angelica-moreira angelica-moreira force-pushed the fix/emit-struct-array-field-write branch from 1a885a6 to cf75491 Compare March 5, 2026 04:40
Generic helper for struct field writes through array index.
Emits update_arr_with arr idx (fun cur -> { cur with field = val }).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@angelica-moreira
Copy link
Copy Markdown
Member Author

I believe I am done. I added update_arr_with to Pulse.Lib.C.Array.fsti, I think it was that you requested, please check :). The emit_assign now emits:

let __val = (!var_val);
update_arr_with (!var_pts) (!var_i) (fun __cur -> { __cur with struct_point__x = __val });

Now we do not have more duplicated arr/idx expressions. The RHS is pre-read into a local since (!var_val) is stateful and can't go inside the lambda.

I also made spec expressions (_ensures,_requires, _invariant) emit Seq.index (value_of arr) (SizeT.v idx) instead of arr.(idx), so I believe postconditions like _ensures(pts[i].x == val) work and verify.

I added a test case like you requested, you can find it here test/struct_array_write.c with three functions (set_x, set_y, set_both), all with pre and postconditions.

…se only syntax. I also added ensures cababilities
@angelica-moreira angelica-moreira force-pushed the fix/emit-struct-array-field-write branch from cff1524 to 4f4f9f5 Compare March 5, 2026 07:26
@gebner
Copy link
Copy Markdown
Contributor

gebner commented Mar 5, 2026

I believe I am done. I added update_arr_with to Pulse.Lib.C.Array.fsti, I think it was that you requested, please check :). The emit_assign now emits:

let __val = (!var_val); update_arr_with (!var_pts) (!var_i) (fun __cur -> { __cur with struct_point__x = __val });

Can you please turn this into:

update_arr_with (!var_pts) (!var_i) (fun __cur __val -> { __cur with struct_point__x = __val }) (!var_val)

I'd really like to avoid generating extra let-bindings here (which might break when we start supporting things like a[i].x = a[j].x = 67).

I also made spec expressions (_ensures,_requires, _invariant) emit Seq.index (value_of arr) (SizeT.v idx) instead of arr.(idx), so I believe postconditions like _ensures(pts[i].x == val) work and verify.

There is no need to do that distinction, and I'd really like to avoid it (to avoid potential divergence in behavior).

I tried removing the is_spec handling, and what breaks is that we were missing parentheses around the Pulse term for ExprT::Index. Please just add the parentheses instead.

I added a test case like you requested, you can find it here test/struct_array_write.c with three functions (set_x, set_y, set_both), all with pre and postconditions.

The test case is great, thanks.

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