Fix usage of internal @check macro#1179
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an UndefVarError in the writebom function caused by the internal @check macro assuming an io variable is in scope when it wasn't. The fix updates writebom to accept an io parameter and adds a compile-time assertion to the @check macro to catch similar issues in the future.
Changes:
- Updated
writebomfunction signature to includeioparameter - Added assertion in
@checkmacro to verifyiois defined in caller's scope - Updated all
writebomcall sites to pass theioargument - Added test case that exercises the fixed code path
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/write.jl | Updated writebom signature to accept io parameter, added assertion to @check macro, and updated all call sites |
| test/write.jl | Added test case for writebom function that exercises the bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1179 +/- ##
==========================================
+ Coverage 90.56% 90.65% +0.09%
==========================================
Files 9 9
Lines 2321 2322 +1
==========================================
+ Hits 2102 2105 +3
+ Misses 219 217 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixes an undefvar error (found by JET.jl on a package that calls
CSV.write(file::String, table::NamedTuple):Repro:
the
@checkmacro assumes certain variables are in scope, namelyio(andbuf,pos,len), but that wasn't true forwritebom.this PR updates
writebomto take a variable namedio, and updates@checkto assert aiovariable is defined.the other variables are always referenced in the expanded code, so we don't
@assert @isdefinedthose.