Skip to content

Move isassigned boundscheck to FSA wrapper#188

Open
jakobnissen wants to merge 1 commit into
JuliaArrays:mainfrom
jakobnissen:isassigned
Open

Move isassigned boundscheck to FSA wrapper#188
jakobnissen wants to merge 1 commit into
JuliaArrays:mainfrom
jakobnissen:isassigned

Conversation

@jakobnissen
Copy link
Copy Markdown

In Base.boundscheck, instead of forwarding boundscheck to the inner vector, do the boundscheck on the wrapper FSA, and then call inner isassigned with @inbounds.
This allows correct results for unsafe inner arrays which does not, or cannot boundscheck.
In general, it would be preferred for FSA to check indices where possible, since the FSA wrapper "owns" the indices.

In Base.boundscheck, instead of forwarding boundscheck to the inner vector,
do the boundscheck on the wrapper FSA, and then call inner isassigned with
@inbounds.
This allows correct results for unsafe inner arrays which does not, or
cannot boundscheck.
In general, it would be preferred for FSA to check indices where possible,
since the FSA wrapper "owns" the indices.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.85%. Comparing base (147851b) to head (b13064e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #188      +/-   ##
==========================================
+ Coverage   92.78%   92.85%   +0.06%     
==========================================
  Files           5        5              
  Lines         208      210       +2     
==========================================
+ Hits          193      195       +2     
  Misses         15       15              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@nsajko nsajko left a comment

Choose a reason for hiding this comment

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

Looking at @code_native, this change does not appear to cause significant changes for the generated code. At least on my platform, Intel AMD64.

Out of courtesy, we should make sure not to commit with a commit message pinging Github user inbounds.

@jakobnissen
Copy link
Copy Markdown
Author

Good sentiment, but @inbounds is a dummy GitHub account set up by JuliaLang - see https://github.com/orgs/JuliaLung/people 😄

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