Disable @inbounds, it's too dangerous for *this* package

[PallHaraldsson]

Explained at Julialang (where the issue was closed):
JuliaLang/julia#45353 (comment)

[KristofferC]

This seem like a gross over reaction to me based on some blog post that found a package with 8 year old code where @inbounds was used incorrectly.

[PallHaraldsson]

Well, not to me if it restores confidence in the Julia ecosystem. It's not just "a package" that's currently (still) broken.

@StefanKarpinski proposed disabling @inbounds entirely, which to me does seem like an overreaction yes, but not proposed here, in case you misunderstood it that way. I still like his proposal though (with the replacement), maybe eventually, why I upvoted there (I noticed you downvoted it).

My original idea was for Julialang, and then as followup, a possible command-line switch to restore current behavior. If done in the package, then such a switch less likely to happen.

https://www.reddit.com/r/Julia/duplicates/uqwd2h/why_i_no_longer_recommend_julia/

got even cross-posted to r/worldnews while later "removed by the moderators" and r/programming and more. I'm trying to limit the (in part, mostly misunderstood) bad effects.

[johnnychen94]

There's no need to overreact like this -- @inbounds might be dangerous if used mistakenly, but it is a useful concept.

Some (broken) packages were written back when custom axes didn't exist -- they still work very well for their originally designed scope, they just don't work well with new stuff.
And this is the nature of evolution.

If we want help, we can put up PR to fix things, to remove or correct unsafe @inbounds usage. But it doesn't mean @inbounds should never be used.

This package doesn't handle @inbounds stuff, so there's nothing to be fixed here.

[johnnychen94]

If you wanna help, maybe you can help review #281 and leave some comments.

[PallHaraldsson]

FYI (the more radical proposal): JuliaLang/julia#45342 (comment)

[ParadaCarleton]

If we want help, we can put up PR to fix things, to remove or correct unsafe @inbounds usage. But it doesn't mean @inbounds should never be used.

Would it make sense to instead create a new @off_inbounds macro and make @inbounds a no-op for OffsetArrays? Users can still skip bounds checks by turning it off with an option; package developers can skip it by using @off_inbounds.

[jishnub]

This will have severe performance implications, since Base functions use @inbounds. Perhaps this is something that Base should introduce, which may be adopted elsewhere

[PallHaraldsson]

Should this be reopened, and done?

Also it's unclear if @inbounds is actually (always) faster, I see talk on it can hindering optimization.

I still believe it's (more) dangerous for non-1-based arrays. With this change all code (using this package) would work, or fail and let you know.

All arrays (using this package) can opt into OffsetArrays.no_offset_view and do I understand correctly then that's a 1-based view, that would be treated as an 1-based array, and then @inbounds could still work on that, e.g. if calling Base.

As documented in this package, some of Julia expects 1-based, and doesn't work otherwise, at least (only parts of?) LinearAlgebra. Does that also apply to Base, would some of it fail with @inbounds there? But at least it could be made to work as I explained.

[jishnub]

Code that expects 1-based indexing should be protected with Base.require_one_based_indexing, as in LinearAlgebra.

My understanding is that @inbounds may hamper performance in general where the code benefits from compiler optimisations such as constant propagation. However, this usually isn't the case in array indexing, so judiciously applied annotations shouldn't hurt performance. Could you provide an example where this isn't the case?

[PallHaraldsson]

Code that expects 1-based indexing should be protected with Base.require_one_based_indexing, as in LinearAlgebra.

Yes, code should, and it likely does, but while I don't have specific code in mind I have generic code (in package) in mind. A lot of them might not think this through.

So each and every one such might break composability in the ecosystem, yes, until found and fixed (but do we want to be unsafe until then, and when can we be sure it is fixed and will not get broken again?). If it were possible to just disable for packages, that would be great, but until then I think better to just do it globally. And well if would make OffsetArrays sort of second class performance wise... but I was thinking, not if you can take a 1-based view? I new macro has been suggested, something like (I added unsafe_) @unsafe_offset_inbounds that has the current semantics, but people, and only those that have thought through, tested with OffsetArrays too should use that...