Repository navigation
Test for closure boxes on Julia 1.14+ - #88
Merged
Merged
Conversation
On Julia 1.14+, use `Test.detect_closure_boxes` to check that no method in ObjectiveC or its submodules captures a variable in a `Core.Box`, which makes that variable type-unstable. This catches regressions where a closure captures a variable that is reassigned. Three boxes existed: `methods(::Class)` reused `meths` after capturing it in a comprehension, `objcm`'s `parse_argument` closure assigned to `value` and `typ` from the enclosing scope and the argument arrays were redundantly reassigned, and `@objcproperties` defined a recursive local function. Use a separate name, a `local` declaration with no reassignment, and a module-level function respectively.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds a
closure boxestestset that runsTest.detect_closure_boxes(ObjectiveC)on Julia versions that have it (1.14+), so that captured variables ending up in aCore.Boxget caught in CI. The function also scans submodules, soFoundation,CoreFoundation,DispatchandOSare covered too. It would also catch regressions like the box inos_log_with_typeon #87.On main it found three boxes, fixed here without changing behavior:
methods(::Class)reassignedmethsafter capturing it in a comprehension, so the second array now has its own name. Inobjcm, theparse_argumentclosure assigned tovalueandtyp, which also exist in the enclosing function, so they're now declaredlocalin the closure. The argument arrays were also reassigned to fresh empty arrays when the method is a plain symbol, which was redundant, so that branch is gone.@objcpropertiesdefinedprocess_propertyas a recursive local function, which is now a module-levelprocess_property!that takes the property list and module as arguments.Tests pass on 1.10, 1.12 and nightly. On nightly the new test fails without the fixes.