Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.37%. Comparing base (cb299dc) to head (b3a3edc).
⚠️ Report is 8 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (cb299dc) and HEAD (b3a3edc). Click for more details.

HEAD has 10 uploads less than BASE

Flag	BASE (cb299dc)	HEAD (b3a3edc)
	13	3

@@             Coverage Diff             @@
##           master     #378       +/-   ##
===========================================
- Coverage   95.70%   74.37%   -21.34%     
===========================================
  Files           6        6               
  Lines         466      359      -107     
===========================================
- Hits          446      267      -179     
- Misses         20       92       +72     

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

@jishnub knows far more about the current state of the package than I do, so I'll defer to him.

For the context on Adapt, my only concern was how it opened everything up for invalidation. Assuming that issue has been resolved, I don't have any objections.

For the context on Adapt, my only concern was how it opened everything up for invalidation

That's been fixed years ago as per your request.

That was my understanding too. I only brought it up because of @vchuravy's mention of past concerns.

We should probably still be careful, though. unwrap_type is based on the not-so-nice WrappedArray union, https://github.com/JuliaGPU/Adapt.jl/blob/944774bb77d1e4059750f65de155dd7a96476e07/src/wrappers.jl#L72-L120, which has some known issues (and is where an AbstractWrappedArray in Base would come in). I haven't seen it cause invalidations, only some more expensive method insertion when loading packages.

Yeah, that's a pretty rough Union!

Referring also to this discussion:
I had a look into the approach that was taken here. And its is not quite so simple, or at least incomplete, if the aim is to make OffsetArrays.jl compatible with CUDA.jl. For example:

julia> using CUDA
julia> a = reshape(cu(collect(1:24)), 2, 3, 4);
julia> sa = OffsetArray(a, (2, 3, 4));
julia> (@view sa[3:4,4:5,5:6]) == a[1:2,1:2,1:2]
ERROR: Scalar indexing is disallowed.

... and supposedly something similar with a ReshapedArray. This means that the broadcasting did not work properly here.
Views in CuArray do not cause scalar indexing. Another issue is

julia> collect(sa)
ERROR: Scalar indexing is disallowed.

These issues can be fixed as shown here, but the disadvantage is that this needs a specific dependency of the extension on CUDA.jl and it is not generically applicable to any wrapped array.

I guess the first problem may be fixed in a better way by implementing extensions to SubArray and ReshapedArray in a similar way. However the other problem seems to stem from some operations in the Julia AbstractArray (?) directly iterating through the array. Is this really faster or more general than implementing these based on the broadcasting mechanism instead?
If so, is there a chance to migrate to broadcasting or implement some mechanism or macro that switches to broadcasting where wanted?

It seems like the following definition is missing in Base?

function Base.Broadcast.BroadcastStyle(W::Type{<:SubArray}) 
    return Base.Broadcast.BroadcastStyle(Adapt.unwrap_type(W))
end

At least, @which on a view of a custom array without a special rule returns only the AbstractArray definition, which itself returns DefaultArrayStyle{2}()