Skip to content

fix: find_call_macro null-deref + macro_call regression tests + task_wrap_up skill#2617

Merged
borisbat merged 1 commit into
masterfrom
bbatkin/macro-call-bugfix-and-coverage
May 9, 2026
Merged

fix: find_call_macro null-deref + macro_call regression tests + task_wrap_up skill#2617
borisbat merged 1 commit into
masterfrom
bbatkin/macro-call-bugfix-and-coverage

Conversation

@borisbat
Copy link
Copy Markdown
Collaborator

@borisbat borisbat commented May 9, 2026

Summary

Three threads from the same dasImgui Phase 0a session, landing together because they're tightly coupled (the bugfix unblocks one of the macro patterns the new tests pin):

  • Bugfix. findModuleCallMacro deref'd the result of Module::findCall without null-checking. For unregistered call_macro names this crashed with EXCEPTION_ACCESS_VIOLATION. Add the null check; find_call_macro(mod, name) is now a safe soft-probe that returns null on miss. (src/builtin/module_builtin_ast_adapters.cpp)
  • Regression coverage. Three new tests under tests/macro_call/:
    • test_find_call_macro_null — pins the bugfix above.
    • test_dyn_register_call_macro — pins the "function_macro registers a per-decl call_macro instance from inside apply()" pattern (what dasImgui's [widget] annotation relies on).
    • test_call_macro_emit_gate — pins the "call_macro gates add_variable on a find_variable miss" pattern (the auto-emit / idempotent module-global trick).
  • Task wrap-up skill + MOUSE FIRST hard rule.
    • New skills/task_wrap_up.md — blind-mouse curation pass for any major task wrap, not just PRs (review the query log, surface the un-asked, cost calculus).
    • skills/make_pr.md §5.5 collapsed to a one-line pointer at the new skill.
    • CLAUDE.md gains a row in the skill table and a "MOUSE FIRST (hard rule)" section at the bottom: mouse__ask is the first tool on any pattern/why/how question, before Explore/Grep/Read.
    • 9 new mouse-data/docs/ entries seeded during the same Phase 0a session: reload firing-order, C++-resource preservation across reload, ImGui backend-data-survives-ctx-roundtrip, plus six macro / daspkg patterns.

The bugfix was already compiled into the local daslang.exe at session start, so the regression tests (4/4) passed against the fix. CI will rebuild from source and validate.

Test plan

  • daslang.exe dastest/dastest.das -- --test tests/macro_call — all 4 tests pass (1 existing + 3 new) in 31 ms, no GC COMPILE LEAK / GC APP LEAK.
  • MCP format_file on all 5 new .das files — already formatted.
  • MCP lint on all 5 new .das files — 0 issues, 0 errors.
  • CI validates AOT against the C++ change (skipped locally — change is a defensive null-check + early return, AOT-compiled callers shouldn't shift).
  • CI validates Sphinx + extended_checks (no doc/RST changes; should be no-op).

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 9, 2026 15:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the AST C++ binding for find_call_macro to avoid a crash on missing names, and adds targeted macro-call regression tests to lock in key call-macro patterns (missing lookup, dynamic registration, and emit-or-reuse gating). It also updates the internal “skills” docs and mouse cache guidance to formalize a task wrap-up curation step and the “MOUSE FIRST” rule.

Changes:

  • Fix findModuleCallMacro to null-check Module::findCall before dereferencing, making find_call_macro a safe “soft probe” on misses.
  • Add tests/macro_call/* regression tests + helpers to pin missing lookup behavior, dynamic call_macro registration from apply(), and find_variable-gated add_variable emission.
  • Add skills/task_wrap_up.md, update skills/make_pr.md, and extend CLAUDE.md + mouse docs to reinforce blind-mouse workflow.

Reviewed changes

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

Show a summary per file
File Description
src/builtin/module_builtin_ast_adapters.cpp Adds a null-check to prevent find_call_macro crashes when the name is not registered.
tests/macro_call/test_find_call_macro_null.das New regression test ensuring missing find_call_macro lookups return null (no crash).
tests/macro_call/_find_call_macro_helper.das Helper call_macro that probes find_call_macro at compile time and folds to a bool.
tests/macro_call/test_dyn_register_call_macro.das New regression test for dynamic per-decl call_macro registration via a function annotation.
tests/macro_call/_dyn_register_helper.das Defines the [dyn_register_42] function_macro that registers a per-function call_macro instance.
tests/macro_call/_dyn_register_decl.das Separate module consuming [dyn_register_42], providing an answer function for the test.
tests/macro_call/test_call_macro_emit_gate.das New regression tests for “emit if absent, reuse if present” module-global gating.
tests/macro_call/_call_macro_emit_gate_helper.das Helper call_macro implementing find_variable + conditional add_variable + ExprVar folding.
skills/task_wrap_up.md New skill doc for blind-mouse wrap-up curation after meaningful work.
skills/make_pr.md Points PR preflight mouse step to the new task wrap-up skill and updates quick reference.
CLAUDE.md Adds the new skill to the table and documents the “MOUSE FIRST” hard rule.
mouse-data/docs/why-does-find-call-macro-crash-with-exception-access-violation-when-called-with-a-name-that-isn-t-registered.md New mouse entry documenting the root cause and fix for the crash.
mouse-data/docs/how-do-i-write-a-regression-test-for-a-daslang-c-binding-bug-that-needs-a-module-pointer.md New mouse entry describing the “use a call_macro as a bridge” testing pattern.
mouse-data/docs/how-do-i-dynamically-register-a-call-macro-instance-from-inside-another-macro-s-apply-or-visit.md New mouse entry documenting dynamic call_macro instance registration pattern + gotchas.
mouse-data/docs/why-can-t-i-use-a-function-macro-annotation-in-the-same-file-that-declares-it.md New mouse entry documenting annotation visibility constraints (same-file usage gotcha).
mouse-data/docs/my-call-macro-errors-with-can-t-locate-variable-ident-before-it-gets-a-chance-to-fire-how-do-i-keep-the-typer-from-pre-resolving.md New mouse entry documenting canVisitArgument to avoid pre-resolution failures.
mouse-data/docs/my-daspkg-package-s-register-native-path-module-isn-t-found-what-s-the-require-path-supposed-to-look-like.md New mouse entry explaining dynamic-module require path rules (group/mod).
mouse-data/docs/what-s-the-exact-firing-order-of-before_reload-shutdown-after_reload-init-during-a-daslang-live-reload.md New mouse entry documenting daslang-live reload lifecycle ordering.
mouse-data/docs/how-do-i-preserve-a-c-owned-resource-across-a-daslang-live-reload.md New mouse entry documenting the persistent-store pointer roundtrip pattern.
mouse-data/docs/does-imgui-s-backend-state-survive-daslang-live-reload-if-i-only-roundtrip-the-imguicontext-pointer.md New mouse entry documenting ImGui backend persistence across reload via ctx-pointer roundtrip.
Comments suppressed due to low confidence (2)

mouse-data/docs/what-s-the-exact-firing-order-of-before_reload-shutdown-after_reload-init-during-a-daslang-live-reload.md:34

  • This document has a duplicated ## Questions section at the end (second heading + repeated question). Removing the duplicate will avoid accidental drift between copies.
- In what context (old or new) do [before_reload] and [after_reload] run?

## Questions
- What's the exact firing order of [before_reload] / shutdown() / [after_reload] / init() during a daslang-live reload, and is is_reload() true throughout?

mouse-data/docs/how-do-i-preserve-a-c-owned-resource-across-a-daslang-live-reload.md:95

  • The ## Questions section is duplicated at the end of the file (second heading + repeated first question). Removing the duplicate keeps the entry clean and prevents divergence.
- Why use a state-check (`if (live_x != null) return`) on init instead of `if (!is_reload())`?

## Questions
- How do I preserve a C++-owned resource (GLFW window, ImGui ctx, audio handle) across a daslang-live reload so the daslang-side state survives?


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

Comment thread tests/macro_call/test_dyn_register_call_macro.das
Comment thread tests/macro_call/test_call_macro_emit_gate.das
Comment thread tests/macro_call/test_call_macro_emit_gate.das
@borisbat borisbat force-pushed the bbatkin/macro-call-bugfix-and-coverage branch from d621097 to 05a34a4 Compare May 9, 2026 15:26
@borisbat borisbat requested a review from Copilot May 9, 2026 15:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

…wrap_up skill

Bugfix
------
findModuleCallMacro deref'd the result of Module::findCall without
null-checking. For unregistered call_macro names this crashed with
EXCEPTION_ACCESS_VIOLATION. Add the null check; find_call_macro is
now a safe soft-probe that returns null on miss.

Macro regression coverage (tests/macro_call/)
---------------------------------------------
- test_find_call_macro_null - covers the bugfix above
- test_dyn_register_call_macro - function_macro that registers a
  per-decl call_macro instance from inside its apply()
- test_call_macro_emit_gate - call_macro that gates add_variable on
  a find_variable miss (auto-emit module global, idempotent)

These tests pin three patterns the dasImgui [widget] macro relies on;
breaking any of them would silently regress dasImgui's call dispatch.

Task wrap-up skill + MOUSE FIRST hard rule
------------------------------------------
- New skills/task_wrap_up.md - blind-mouse curation pass for any
  major task wrap, not just PRs. Three steps: review log,
  surface the un-asked, cost calculus.
- skills/make_pr.md section 5.5 collapsed to a pointer at task_wrap_up.md.
- CLAUDE.md gains a row in the skill table and a "MOUSE FIRST
  (hard rule)" section at the bottom: mouse__ask is the first tool
  on any pattern/why/how question, before Explore/Grep/Read.

Plus 9 mouse-data/docs/ entries seeded during dasImgui Phase 0a:
reload firing-order, C++-resource preservation across reload,
ImGui backend-data-survives-ctx-roundtrip, plus six macro / daspkg
patterns surfaced during the same session.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@borisbat borisbat force-pushed the bbatkin/macro-call-bugfix-and-coverage branch from 05a34a4 to acaad72 Compare May 9, 2026 15:41
@borisbat borisbat merged commit a03ae02 into master May 9, 2026
29 checks passed
pull Bot pushed a commit to forksnd/daScript that referenced this pull request May 9, 2026
…in linux_arm CI

build.yml no longer uploads `daslang-<platform>-<arch>.zip` (release.yml is the
sole release publisher) — also kills the empty-name `daslang--.zip` that
sanitizer rows produced via clobbering uploads.

linux_arm Release in build.yml now enables `dasHV` and `dasSQLITE` to mirror
release.yml's module set, so the LLVM AArch64 SelectionDAG hang seen in the
v0.6.2-RC2 release run reproduces in regular CI. The linux_arm test step
drops `--failures-only` on JIT so PASS lines name each test as it finishes —
the file running just before the hung one will be the last PASS printed.

Also threads the five post-RC2 PRs (GaijinEntertainment#2611, GaijinEntertainment#2612, GaijinEntertainment#2614, GaijinEntertainment#2616, GaijinEntertainment#2617) into
CHANGELIST.md and the site/index.html change list, with a May 9 News entry.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@borisbat borisbat deleted the bbatkin/macro-call-bugfix-and-coverage branch May 14, 2026 16:00
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