Feature/split serialize

[aleksisch]

PR #2460 — feature/split-serialize

Summary

This PR fixes AST serialization roundtrip correctness and re-enables --ser/--deser CI testing.

Architectural changes

Virtual dispatch for AST nodes (compiler: add virtual dispatch)

Added Expression::dispatch(Visitor&) — a non-recursive virtual shim that routes a single node to the most-specific visitor overload for its concrete type. Previously the only way to drive visitor logic was the full recursive visit() traversal; now a single node can be processed in isolation.

Serialize extracted from AST (compiler: extract serialize from ast)

Expression::serialize (and related serialize methods on expression subclasses) moved out of ast_expressions.h / ast.h into src/builtin/module_builtin_ast_serialize.cpp as a SerializeVisitor that uses the new dispatch shim. AST types no longer include serialization headers or carry serialization logic — the compiler core is now independent of the serializer.

Correctness fixes

Stable node IDs (serialization: stable id in serialization)

The dedup maps in AstSerializer previously keyed on raw uintptr_t(ptr). Pointer addresses are reused across Program lifetimes and during short-lived AST construction, causing two logically-distinct nodes to collide on the same key — the writer deduped them into one, the reader reconstructed only one, and tag() tripped on a misaligned hash word. Maps now key on SerializeNodeId{ptr, epoch} (raw pointer + per-serialize-call epoch), which eliminates cross-call collisions.

g_Program stale pointer after deserialization

finalizeModule sets g_Program to a temporary Program then calls thisModule.release(), leaving g_Program pointing at a program with null thisModule. Fixed by saving/restoring g_Program around the block invocation in rtti_ast_serializer_deserialize_program so that compiling_module() sees the correct module during simulate and test execution.

Function::fromGeneric now serialized

fromGeneric was commented out in Function::serialize. After deserialization, qm_call_name_matches in daslib/ast_match couldn't match generic call names because the fallback call.func.fromGeneric.name path was always null. fromGeneric is now serialized via the existing FunctionPtr dedup path.

Macro module context leak

Non-promoted [call_macro] modules get a macroContext (smart_ptr, rc=1) during finalizeModule. Program::library is plain ModuleLibrary whose destructor does not delete modules, so those modules and their contexts were abandoned. Fixed by calling prog->thisModule.release() + prog->library.reset() after the block invocation returns.

Test infrastructure

Expected-failure file naming unified (serialization: unify failed tests naming)

Nine test files that were expected to fail compilation but didn't follow the failed_/cant_/invalid_ prefix convention were renamed. dastest's --ser pass now skips files with these prefixes (nothing to serialize for a program that doesn't compile). Naming convention documented in tests/README.md.

CI re-enabled (serialization: stable id in serialization)

extended_checks.yml gains a Test ser/deser step that serializes the full test suite then deserializes and re-runs it. Previously dropped because --deser crashed inside AstSerializer::tag.

[Copilot]

Pull request overview

Refactors AST expression serialization by removing per-node serialize(AstSerializer&) virtual methods from Expression subclasses and replacing them with a visitor-based serialization path driven by a new Expression::dispatch(Visitor&) shim.

Changes:

• Replace Expression::serialize/ExprXxx::serialize with Expression::dispatch/ExprXxx::dispatch and route expression serialization through SerializeVisitor.
• Add src/ast/ast_dispatch.cpp implementing dispatch for expression classes.
• Wire the new dispatch TU into the build and update headers accordingly.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/builtin/module_builtin_ast_serialize.cpp	Moves expression serialization into SerializeVisitor and updates AstSerializer::operator<<(ExpressionPtr&) to use dispatch.
src/ast/ast_dispatch.cpp	Adds per-class dispatch shims to call the correct Visitor::preVisit(...) overloads.
include/daScript/ast/ast_expressions.h	Replaces serialize overrides with dispatch declarations for expression types.
include/daScript/ast/ast.h	Replaces Expression::serialize with Expression::dispatch and forward-declares Visitor.
CMakeLists.txt	Adds src/ast/ast_dispatch.cpp to AST_SRC.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 22 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/builtin/module_builtin_ast_serialize.cpp:900

• serializeSmartPtr is still keyed on uint64_t, but getSerializeId(obj.get()) now returns SerializeNodeId. As written this won’t compile (type mismatch) and also doesn’t match the updated smart*Map member key types. Update serializeSmartPtr (both declaration and definition) to use SerializeNodeId for id and for the objMap key type, and adjust the null check accordingly (e.g., id.ptr == nullptr).

    // This method creates concrete (i.e. non-polymorphic types without duplications)
    template<typename T>
    void AstSerializer::serializeSmartPtr( smart_ptr<T> & obj, das_hash_map<uint64_t, smart_ptr<T>> & objMap) {
        uint64_t id = getSerializeId(obj.get());
        *this << id;
        if ( id == 0 ) {
            if ( !writing ) obj = nullptr;
            return;
        }

[aleksisch]

Pull request overview

Copilot reviewed 13 out of 22 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)

src/builtin/module_builtin_ast_serialize.cpp:900

* `serializeSmartPtr` is still keyed on `uint64_t`, but `getSerializeId(obj.get())` now returns `SerializeNodeId`. As written this won’t compile (type mismatch) and also doesn’t match the updated `smart*Map` member key types. Update `serializeSmartPtr` (both declaration and definition) to use `SerializeNodeId` for `id` and for the `objMap` key type, and adjust the null check accordingly (e.g., `id.ptr == nullptr`).

    // This method creates concrete (i.e. non-polymorphic types without duplications)
    template<typename T>
    void AstSerializer::serializeSmartPtr( smart_ptr<T> & obj, das_hash_map<uint64_t, smart_ptr<T>> & objMap) {
        uint64_t id = getSerializeId(obj.get());
        *this << id;
        if ( id == 0 ) {
            if ( !writing ) obj = nullptr;
            return;
        }

Valid, removed this method at all. It's unused.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 22 changed files in this pull request and generated 3 comments.