Add composable serialization intrinsics - #1224
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: ff48f039f0
ℹ️ 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".
| && ((idParameter != null && access.getVarName().equals(idParameter)) | ||
| || access.getVarName().equals(nameParameter) || access.getVarName().equals(valueParameter))) { |
There was a problem hiding this comment.
When a schema-aware callback contains a nested closure with an ordinary local named id—for example, () -> begin let id = 100; use(id) end—the outer shadowing check intentionally skips that closure, but this visitor replaces the local read because its block/local tracking only recognizes nameParameter and valueParameter. The read therefore becomes the current field's schema ID, changing runtime behavior; include idParameter in both local-shadow tracking paths. Only __wurst-prefixed nested callback locals are documented as unsupported.
AGENTS.md reference: AGENTS.md:L307-L309
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: 9c5de3918b
ℹ️ 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".
| field.declaration.addError("Schema-aware field iteration requires @saveField(id) on field " | ||
| + field.key() + "."); |
There was a problem hiding this comment.
When a caller uses the new three-parameter form, this branch refuses to lower a class unless every included field carries the compiler-recognized @saveField annotation, while schemaId also special-cases @saveFields. This makes compiler semantics depend directly on serialization-format metadata, contrary to the repository contract that field iteration remain general-purpose and contain no save-format knowledge; keep ID policy in the serialization library instead of recognizing these annotations in SyntacticSugar.
AGENTS.md reference: AGENTS.md:L298-L301
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: 0ae98c79cd
ℹ️ 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".
| if (CompilerIntrinsics.isDeclaration(candidate.getDef())) { | ||
| continue; |
There was a problem hiding this comment.
When an imported @compilerintrinsic declaration and an applicable ordinary closure overload are both visible, this skip makes hasApplicableUserFunction disable intrinsic lowering, but normal overload resolution still retains the annotated declaration. I reproduced this with matching imported forFields(IntCallback) declarations: the call reports both as ambiguous instead of selecting the ordinary function. Filter the intrinsic contract from subsequent resolution whenever an ordinary candidate wins; the same issue applies to mapFields and newInstance.
AGENTS.md reference: AGENTS.md:L310-L311
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: 54131e219f
ℹ️ 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".
| statements.remove(targetVariable); | ||
| statements.clearAttributes(); | ||
| targetVariable = null; | ||
| targetName = targetAccess.getVarName(); |
There was a problem hiding this comment.
When mapFields targets a tuple local and the callback contains a nested closure whose parameter reuses the tuple variable's name, replacing the fresh temporary with only targetAccess.getVarName() makes substituted field-value accesses bind to that nested parameter. For example, mapping payload with an inner (Pair payload) -> value + payload.left reads both operands from the inner tuple and writes incorrect values to the outer tuple; retain a capture-free target reference and write the result back instead. Ordinary nested callback names are supported—the documented exception is limited to __wurst-prefixed names.
AGENTS.md reference: AGENTS.md:L308-L310
Useful? React with 👍 / 👎.
| public static boolean isDeclaration(de.peeeq.wurstscript.ast.FunctionDefinition definition) { | ||
| return definition.attrHasAnnotation(ANNOTATION); |
There was a problem hiding this comment.
When an IDE resolves an annotated wurstNewInstance<T>() call, this declaration is excluded from ordinary-function detection, after which AttrFuncDef.calculate(ExprFunctionCall) returns null immediately because CompilerIntrinsics.isNew(node) is true. Consequently get-definition and hover return no result for the documented construction intrinsic, even though the new declaration mechanism works for the field-iteration intrinsics; preserve the declaration link for tooling while keeping compiler lowering intact.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Adds the minimal compiler surface needed for a composable standard-library serialization framework while keeping serialization policy and runtime metadata in the library.
@compilerintrinsicdeclarations for canonicalwurstForFields,wurstMapFields, andwurstNewInstance<T>()APIs.wurstNewInstance<T>()declaration link for hover and definition navigation without changing intrinsic lowering.Why the
wurstprefixThe canonical names use a Wurst-style prefix instead of the earlier double-underscore convention. This makes accidental collisions much less likely while remaining readable and discoverable. The resolver fix also makes collisions deterministic: an applicable ordinary overload wins over an imported intrinsic contract for all three operations.
Serialization composition
The compiler exposes field structure and concrete field types through per-field callback specialization. A library can overload codec calls for primitives, nested opted-in classes, tuples, enums, nullable values, and generic collections such as
ArrayList<T>. Stable persisted field IDs remain an explicit library concern rather than compiler-generated save metadata.Backend constraints
Both JASS and Lua emit direct accesses/construction. The change adds no runtime reflection, class-name lookup, global type-ID dispatch, uninitialized allocation, or broad Lua monomorphization.
Tests
FieldIterationTests: JASS/Lua lowering, explicit targets, tuples, nested/composite field kinds, inherited/module fields, generic construction, diagnostics, compatibility spellings, ordinary-overload precedence, and nested-closure tuple capture safety.wurstNewInstance<T>().gradlew test: passed ond43e35954in 10m24s.