docfix reinterpret #502
docfix reinterpret #502
Conversation
docs/src/pages/api.md
Outdated
| ``` | ||
| If you absolutely insist on obtaining a `Vector{<:SVector}` referencing the same memory | ||
| and don't mind a maintenance nightmare and data-corrupting tbaa-related bugs appearing in | ||
| julia 1.1 or later, then you can obtain this by |
There was a problem hiding this comment.
😆
This might seem a little confusing to the uninitiated. I wonder if we should write this in a slightly more factual way?
|
Friendly bump. The CI fail ("The remote server returned an error: (404) Not Found.") has nothing to do with this PR, and I'd guess some docfix should go into 1.0 #532. |
|
bump |
There was a problem hiding this comment.
Thanks for this! I suggest trimming the last section because I really don't think we should encourage people to write code with undefined behavior. Even if it happens to work in the latest julia.
If ReinterpretArray is still slow, that needs to be fixed in Base. If nobody has the time to work on that, I suggest a workaround package FastReinterpret (or something) which knows how to pull the necessary dirty tricks and advertises how to use it somewhat safely. I assume there's nothing StaticArrays-specific about reinterpret being too slow.
| Julia 0.7 and 1.0, this violates the language specifications about aliasing | ||
| (Arrays with different `eltype` must not share memory), and is to be considered a dirty hack. | ||
| This is nevertheless safe on 1.0.* or earlier versions of the Julia compiler, but will violate | ||
| compiler assumptions on Julia 1.1 or later, and cause subtle data-corrupting miscompilation bugs. |
There was a problem hiding this comment.
If this is so broken, let's not suggest doing it in the docs! (I suggest we just delete this section ;-) )
| function svectors(x::Matrix{T}, ::Val{N}) where {T,N} | ||
| size(x,1) == N || throw("sizes mismatch") | ||
| isbitstype(T) || throw("use for bitstypes only") | ||
| reinterpret(SVector{N,T}, reshape(x, length(x))) |
There was a problem hiding this comment.
We can use vec(x) rather than reshape here.
| size(x,1) == N || throw("sizes mismatch") | ||
| isbitstype(T) || throw("use for bitstypes only") | ||
| res = Vector{SVector{N,T}}(undef, size(x,2)) | ||
| GC.@preserve res x ccall(:memcpy, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, Csize_t), pointer(res), pointer(x), sizeof(x)) |
There was a problem hiding this comment.
This seems unnecessarily low level for the documentation. How about simply using
copy(reinterpret(SVector{N,T}, vec(x)))
If you do think we need this low level optimization perhaps we should be providing some version of svectors as part of the official API rather than suggesting users do these low level hacks themselves? (#496 I guess)
Partially resolves #501.