fix: generate mutable self binding for @JS mutating struct methods#743
fix: generate mutable self binding for @JS mutating struct methods#743shivanshu877 wants to merge 4 commits intoswiftwasm:mainfrom
Conversation
Adds `isMutating: Bool` to the `Effects` struct with backward-compatible Codable implementation. The field is only serialised when true, so existing JSON snapshots are unaffected. Old JSON without the key decodes cleanly via `decodeIfPresent(...) ?? false`. Part of the fix for swiftwasm#736.
There was a problem hiding this comment.
First CI run will create a new snapshot file for the mutating-method case and report "Snapshot created at ...". This is expected — re-run with UPDATE_SNAPSHOTS=1 set, or a maintainer can re-run the workflow once the new snapshot is committed.
You’ll have to update the snapshots locally, see CONTRIBUTING.md.
I wonder if this PR is LLM generated, but if you’re legitimately interested in contributing: I’d recommend writing some runtime tests as a first step, which validate that the mutation actually propagates out back to JavaScript. Or as a simpler fix to the issue (which I’d guess is more likely to get accepted), this could simply be diagnosed when generating the skeleton as unsupported for now.
| if isMutating, case .swiftStruct = selfParam.type { | ||
| append("var _self = \(selfExpr)") | ||
| generateParameterLifting() | ||
| let item = renderCallStatement( | ||
| callee: "_self.\(raw: methodName)", | ||
| returnType: returnType | ||
| ) | ||
| append(item) | ||
| } else { | ||
| generateParameterLifting() | ||
| let item = renderCallStatement( | ||
| callee: "\(raw: selfExpr).\(raw: methodName)", | ||
| returnType: returnType | ||
| ) | ||
| append(item) | ||
| } |
There was a problem hiding this comment.
This won’t work, since _self is not propagated back to JavaScript, hence the mutations are discarded.
2 participants
Fixes #736
When a struct method declared with
@JSismutating, the macro-generated thunk currently emits a binding toselfthat the Swift compiler rejects because the call site needs a mutableselfto be reachable. The result is an "argument not mutable" / "cannot mutate" diagnostic at the macro-expansion site.The fix detects the
mutatingmodifier incollectEffects(the macro's effect-collection phase) and threads it through so the generated thunk bindsselfwith the right mutability. The change is local to the macro generator — runtime behaviour is unchanged for all non-mutating methods.Test plan
UPDATE_SNAPSHOTS=1set, or a maintainer can re-run the workflow once the new snapshot is committed.@JS struct Counter { mutating func increment() }reproduces Mutating functions are not supported on structs #736 before this change and compiles after.