Add compile-time field iteration for serialization - #1219
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9e2100348
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| String name = call.getFuncName(); | ||
| if ("forFields".equals(name) || "mapFields".equals(name)) { | ||
| calls.add(call); |
There was a problem hiding this comment.
Any existing package that defines a regular unqualified forFields or mapFields function now has every call collected before name resolution. For example, function forFields(int x) ... followed by forFields(1) is reported as being outside an instance context instead of resolving to the user function, introducing a source-breaking global reservation of two ordinary identifiers. Restrict the rewrite to explicit intrinsic syntax or markers so resolvable user calls remain valid.
Useful? React with 👍 / 👎.
| public void visit(ExprVarAccess access) { | ||
| super.visit(access); | ||
| if (access.getVarName().equals(nameParameter) || access.getVarName().equals(valueParameter)) { | ||
| accesses.add(access); |
There was a problem hiding this comment.
When the callback body contains a nested closure whose parameter reuses name or value, this visitor descends into that closure and replaces its bound ExprVarAccess solely by spelling. For example, forFields((name, value) -> consume((int value) -> value)) silently changes the inner closure to return this.<field> rather than its argument for every field, miscompiling valid code; make substitution scope-aware and do not replace references shadowed by nested declarations.
Useful? React with 👍 / 👎.
| @Test | ||
| public void serializesAndDeserializesFieldsWithoutRuntimeReflection() throws IOException { | ||
| test() | ||
| .testLua(true) |
There was a problem hiding this comment.
This test calls .testLua(true), whose setter also sets luaOnly = true in WurstScriptTest, so the harness skips every Jass translation/interpreter variant. The new compiler feature therefore has no committed regression coverage on Jass; enable Lua without luaOnly or add a separate Jass execution test.
AGENTS.md reference: AGENTS.md:L215-L221
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d78b456b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int statementIndex = statements.indexOf(call); | ||
| statements.remove(statementIndex); |
There was a problem hiding this comment.
When either placeholder has an explicit type, removing the call also removes its WShortParameter nodes before validation, so invalid annotations are silently accepted. For example, __wurst_mapFields((NoSuch name, NoSuch value) -> value) compiles and runs despite both referenced types being undefined. Validate these declarations or reject explicit annotations before discarding the closure so ordinary type diagnostics are not bypassed.
AGENTS.md reference: AGENTS.md:L84-L85
Useful? React with 👍 / 👎.
1 participant
What changed
Adds two compile-time-expanded field operations for instance methods and constructors:
__wurst_forFieldsemits one direct callback expression per instance field.__wurst_mapFieldsemits one direct assignment per field using the callback result. Static fields are excluded and declaration order is preserved. The__wurst_spelling makes the compiler intrinsic explicit and leaves ordinary user-definedforFields/mapFieldsfunctions untouched.Why
Wurst save/load libraries currently require handwritten serialization and deserialization code for every field. Runtime reflection would add metadata and lookup overhead on Warcraft III targets. Expanding field operations before type checking gives reflection-like ergonomics while preserving overload resolution and producing the same direct field accesses as handwritten code.
This is intentionally a narrow draft/prototype so the API and limitations can be discussed before broadening it (for example inheritance, module fields, opt-out annotations, or schema/version behavior).
Impact
Validation
./gradlew test, 8m40s).git diff --checkpasses.