Keep the bounds test's unchecked reads inside an array it owns

[nelson2005]

test_jit_options_reach_the_is_null_decorators failed on the Windows cell of upstream's main after Goykhman#12 went in (job) and passed when it was re-run. The child that runs without bounds checking exited 0xC0000005, an access violation.

The script built a one-byte bitmap and asked for bit 100000, so without bounds checking is_null and unpack_booleans read 12.5 KB past the array, and the test asserted that they returned. What lies behind a one-byte array is the allocator's business. It was readable in 86 of the 87 Windows jobs that have run the test, here and upstream.

Now the bitmap is the first byte of an array that holds every byte the calls reach. With bounds checking on both calls still raise IndexError, the check being against the bitmap's one byte. With it off they read memory the array owns. The test still fails when the options stop reaching the decorators in is_null.py, and the crash reproduces on Linux with the bitmap on the last byte of a page and unreadable pages behind it.

The test's comment named all three of is_null.py's decorators and its script exercised two, and of the options it observed only the two that show. It now checks the cache option on each of the three, off and on, and reads nogil, which shows on none of them, back from the three dispatchers. The catalogue has an entry per decorator, each hard-coded uncached, and two more that only the new legs catch: the cache option dropped for unpack_booleans alone, and every option but cache and boundscheck dropped.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

MiniMax Code Review

Let me analyze this pull request to understand what changes are being made and why.

Summary of Changes

1. .github/scripts/mutation_guard_check.py (modified)

This file appears to be a mutation testing guard - it detects when code changes might break existing tests by checking for specific patterns. The modifications add three new "mutation guards":

1. is_null's decorator stops reading the options - Checks if the is_null function's decorator stops forwarding options like jit_with_options to njit.

2. unpack_booleans' decorator stops reading the options - Same check for unpack_booleans function.

3. is_null_struct's decorator stops reading the options - Same check for is_null_struct function.

4. the cache option stops reaching unpack_booleans while every other option does - Checks that the cache option doesn't reach unpack_booleans.

5. the decorators stop forwarding any option but cache and boundscheck - Checks that decorators only forward cache and boundscheck options.

2. test/test_cache.py (modified)

This adds tests to verify that JIT options (particularly boundscheck, cache, and nogil) properly reach the decorators in is_null.py:

1. Changed CHECK_BOUNDS - The bitmap is now larger (100000 // 8 + 1 bytes) but sliced to 1 byte. This is a clever way to ensure there's actual memory allocated beyond the array bounds, which would trigger an IndexError if bounds checking is enabled.

2. Added SHOW_NOGIL - Tests that the nogil option is properly forwarded to the three functions.

3. New test test_jit_options_reach_the_is_null_decorators - Verifies that:

• Bounds checking works correctly when enabled and disabled
• Cache files aren't created when cache is False
• Cache files are created for all three functions when cache is True
• nogil option is properly forwarded to all three functions

The key insight here is that the test suite was previously passing despite the code having bugs because:

• The "options test" only imported viewers, not is_null, unpack_booleans, or is_null_struct
• The hardcoded options on those decorators kept things working
• There was no test for the boundscheck contract

The changes fix this by adding proper tests and fixing the decorators to correctly forward all options.

Looking at the mutation guard changes, I need to verify what they're actually checking. The guards validate that is_null, unpack_booleans, and is_null_struct decorators are reading options from jit_with_options rather than using hardcoded values, and that the cache option doesn't incorrectly apply to unpack_booleans.

There's also a guard checking that decorators only forward cache and boundscheck options while filtering out others like nogil. The test modifications around CHECK_BOUNDS and SHOW_NOGIL appear to be validating that these option forwarding rules work correctly in practice.

Code Review: PR for JIT Options Forwarding

Overview

This PR adds mutation guards and tests to ensure that JIT options (boundscheck, cache, nogil) are properly forwarded from decorators in numbarrow/core/is_null.py to the underlying njit calls, rather than being hardcoded or dropped.

Issues Identified

1. Mutation Guard for "unpack_booleans" Has Trailing Whitespace Issue

In the third test case (line 726), there's a potential issue with the assertion in the mutation guard:

'            return njit(signature, **{jit_options, "cache": jit_options.get("cache")'

This looks syntactically incorrect - {jit_options, ...} would be a set literal, not a dict. It should likely be:

'            return njit(signature, **{**jit_options, "cache": jit_options.get("cache")'

2. Test Bitmap Allocation Is Clever but Potentially Fragile

"bitmap = np.zeros(100000 // 8 + 1, dtype=np.uint8)[:1]\n"

This creates a 12,501-byte buffer but only exposes 1 byte. The comment explains why:

• With bounds checking: reads past the 1-byte view into the allocated 12,501 bytes → IndexError
• Without bounds checking: reads past the 12,501 bytes into unmapped heap memory → potential crash

Concern: This relies on memory layout behavior that may not be guaranteed across different Python implementations, Numba versions, or memory allocators. While the comment explains the rationale well, consider:

1. Adding a platform-specific skip (e.g., pytest.mark.skipif(sys.platform == 'win32', ...))
2. Or using a more deterministic approach like a mock

3. Cache Test Verification Could Be Stronger

assert _index_files(tmp_path / "checked") == [] and _index_files(tmp_path / "unchecked") == []

This assertion combines two conditions with and. If the first part fails, the error message might be confusing. Consider splitting:

assert _index_files(tmp_path / "checked") == []
assert _index_files(tmp_path / "unchecked") == []

4. Missing Error Handling in Test

The _run() function's output is used directly without checking for exceptions in the child process. While there's returncode checking, stderr isn't always shown on failure:

# Good:
assert checked.returncode == 0 and checked.stdout.split() == ["IndexError", "IndexError"], checked.stderr

# But for the nogil test, stderr isn't included:
assert forwarded.returncode == 0 and forwarded.stdout.split() == ["True", "True", "True"], forwarded.stderr

This is inconsistent - consider always including stderr in assertion messages.

5. Comment Mentions "dispatchers" Without Explanation

# An option that shows on none of them, nogil, is read back from the three
# dispatchers, so a decorator forwarding only the two that show is caught
# as well.

The term "dispatchers" isn't defined in the codebase visible here. Consider adding a brief explanation or reference.

Positive Aspects

1. Well-documented test cases - The comments explain the reasoning clearly
2. Comprehensive coverage - Tests boundscheck, cache, and nogil options
3. Clever use of mutation guards - Catches regressions where decorators stop forwarding options
4. Proper cleanup - Tests verify no cache files left behind when disabled

Suggested Fixes

# Fix #1: mutation_guard_check.py line ~726
# Change:
'            return njit(signature, **{jit_options, "cache": jit_options.get("cache")'
# To:
'            return njit(signature, **{**jit_options, "cache": jit_options.get("cache")'

# Fix #3: Split assertion in test_cache.py
# Change:
assert _index_files(tmp_path / "checked") == [] and _index_files(tmp_path / "unchecked") == []
# To:
assert _index_files(tmp_path / "checked") == [], _index_files(tmp_path / "checked")
assert _index_files(tmp_path / "unchecked") == [], _index_files(tmp_path / "unchecked")

Conclusion

The PR addresses a real bug where JIT options were being dropped by decorators. The mutation guards are a clever way to prevent regression. The main concerns are the potentially fragile memory-layout test and the syntax issue in the mutation guard. After addressing those, this should be good to merge.

[nelson2005]

From the fake Slim Shady:

On the review of a683dd3, its five points.

1. The entry reads **{**jit_options, "cache": ...} (line 728), a dict. The catalogue applied it and ran the suite on it, 123 of 123 killed.
2. It is the other way round. With bounds checking off the reads stay inside the array: its 12501 bytes hold byte 12500, the highest either call reaches. With it on, the check is against the one-byte view. Nothing reads past the allocation on any platform, which is what the change is for.
3. pytest's assertion rewriting prints each side of the and, so the message names the leg that failed.
4. That assertion carries forwarded.stderr (line 339).
5. A dispatcher is what @njit returns, numba.core.dispatcher.Dispatcher, whose targetoptions the script reads.

Nothing to change.

[nelson2005]

From the fake Slim Shady:

Closing unmerged. Everything here landed upstream through Goykhman#13, merged as 2af2c6d, and fork main takes it through the sync/upstream-2026-10-01 PR. This PR was the CI and bot gate for that work.