-
Notifications
You must be signed in to change notification settings - Fork 0
Improve @with_pool Macro Source Location for Better Coverage & Debugging
#11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ase 1 RED) Add test infrastructure for LineNumberNode improvements: - 3 helper functions: find_linenumbernode_with_line, has_valid_linenumbernode, get_function_body - 8 test cases for source location verification - 4 tests currently FAIL (expected): function forms point to macros.jl instead of user file - 4 tests PASS: block forms already work correctly RED phase complete - tests verify that function body's first LNN should point to call site, not macros.jl. This will be fixed in Phase 2 with __source__ integration.
…rage/debugging - Add source location helper functions: - _maybe_add_source_location! for block form LNN insertion - _ensure_body_has_toplevel_lnn for function form LNN replacement - Update code generation functions with source kwarg: - _generate_pool_code - _generate_pool_code_with_backend - _generate_function_pool_code - _generate_function_pool_code_with_backend - Apply _ensure_body_has_toplevel_lnn to new_body AFTER quote block creation to replace macros.jl LNN with user call site LNN - Update all 8 macro definitions to pass __source__: - @with_pool (4 variants) - @maybe_with_pool (4 variants) Benefits: - Codecov correctly maps macro-generated code to user source - Stack traces show user file:line instead of macros.jl:XXX - Debugger breakpoints work at user code locations All 41 Source Location Preservation tests now pass.
…tions Add _fix_try_body_lnn! helper that recursively replaces macros.jl LineNumberNodes inside try blocks with user source LNNs. Julia determines stack trace line numbers from the LAST LNN before the error location. Previously, try block LNNs from macros.jl (line 631) appeared in stack traces. Now user code lines are shown correctly. Before: test_func @ none:630 (macros.jl line) After: test_func @ none:7 (actual error line) Add Test 10 to verify stack trace line number accuracy.
…unctions - Remove dead code: _maybe_add_source_location! (never called) - Add _find_first_lnn_index helper to scan first 3 args for LNN - Update _ensure_body_has_toplevel_lnn to handle Expr(:meta, ...) from @inline - Update _fix_try_body_lnn! to handle meta expressions - Fix hardcoded line number checks (< 600) → use relative comparison - Add Test 12: @inline function AST source location verification This fixes issues where @inline macro inserts Expr(:meta, :inline) before LineNumberNode, causing the previous args[1] check to miss the actual LNN.
- Add source.file === :none guard to _ensure_body_has_toplevel_lnn and _fix_try_body_lnn! to prevent clobbering valid file info in REPL/eval - Increase max_scan from 3 to 6 for multiple stacked macro metadata - Add find_toplevel_lnn test helper for meta-aware LNN scanning - Remove unused expected_line variable from Test 12 - Add Test 13: @inline outer macro function source location - Add Test 14: @inbounds outer macro block source location
- _find_first_lnn_index: use enumerate() instead of manual indexing - find_toplevel_lnn (test): use direct iteration since only value needed - Improve docstrings with example AST prefix patterns
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11 +/- ##
==========================================
- Coverage 97.08% 96.41% -0.68%
==========================================
Files 8 8
Lines 961 1004 +43
==========================================
+ Hits 933 968 +35
- Misses 28 36 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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 enhances the @with_pool and @maybe_with_pool macros by implementing proper source location tracking through __source__ and LineNumberNode manipulation. This ensures that stack traces, coverage reports, and debugging tools point to user code rather than internal macro implementation files.
Key Changes:
- Added
__source__parameter propagation through all macro entry points - Implemented helper functions to manage
LineNumberNodeplacement in generated AST - Added comprehensive test suite to verify source location preservation across different macro usage patterns
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/macros.jl | Added source location helper functions (_find_first_lnn_index, _ensure_body_has_toplevel_lnn, _fix_try_body_lnn!) and integrated source parameter into all macro definitions and code generation functions |
| test/test_macro_expansion.jl | Added 388 lines of tests including helper functions for AST inspection and comprehensive test cases covering block forms, function forms, backend variants, stack trace verification, and nested/edge cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove outdated reference to non-existent _maybe_add_source_location! - Clarify line number validation comments (relative distance, not absolute)
Summary
This PR improves
@with_pooland@maybe_with_poolmacros by integrating__source__andLineNumberNodehandling to provide:macros.jlProblem
Before This PR
Issues:
macros.jlinstead of user's source fileAfter This PR
Verification
Quick Test