remove all uses of @pure

[oscardssmith]

Most of these were invalid uses of @pure and almost all of the rest were unnecessary.

[thchr]

almost all of the rest were unnecessary

I don't completely understand this: doesn't this imply that some of them were necessary in some capacity? In the diff, unless I missed something, it looks like they were all removed?

Although I understand that automatic effects analysis and constant propagation can now figure much of this out on its own, this change seems to assume that it can figure all of these cases out now: shouldn't we verify this?
I had imagined some of these should've been converted to a more specific form of @assume_effects rather than completely removed?

Xref: related PR to *Core JuliaArrays/StaticArraysCore.jl#17 and earlier discussion JuliaArrays/StaticArraysCore.jl#1 (comment). in particular, I think the "should be sanity checks" bit from your earlier comment is important and not clear at present.

[oxinabox]

Since @mateuszbaran asked on slack:

Which ones are wrong? Which ones are not necessary? What could go wrong when the usage is wrong, given that this is one of the most widely used Julia package and no issue has been linked to incorrect use of @pure? Should some of them be replaced by @assume_effects? About 2/3 of these cases are fairly clear to me but the rest requires obscure knowledge. Is there something wrong with @pure (::Type{Tuple})(::Size{S}) where {S} = S? Or @pure size_tuple(::Size{S}) where {S} = Tuple{S...}? Or @pure has_eltype(::Type{<:StaticArray{<:Tuple,T}}) where {T} = @isdefined T? If it's clear to you, I'd like to know. I could probably spend hours and figure it out eventually but why bother if nothing is broken?

And since my answer is long, i thought i would just post it here.
(sorry if it is a bit of a rant, I got two hours sleep and might not be phrasing things well)

Which ones are wrong?

Every singe one of them is wrong except:

• https://github.com/JuliaArrays/StaticArrays.jl/pull/1130/files#diff-2c01b8241456404adf292259053462c729c6724f72cbc0ad1ea1295619c63e06R90-R91
• https://github.com/JuliaArrays/StaticArrays.jl/pull/1130/files#diff-348ed02dc6a8ae9a9040d93373b263d92798a4bc358c664b9278c751a100e345R41
• https://github.com/JuliaArrays/StaticArrays.jl/pull/1130/files#diff-348ed02dc6a8ae9a9040d93373b263d92798a4bc358c664b9278c751a100e345R49-R50
• https://github.com/JuliaArrays/StaticArrays.jl/pull/1130/files#diff-348ed02dc6a8ae9a9040d93373b263d92798a4bc358c664b9278c751a100e345R56-R58

And those ones are all trivial so it is "obvious" @pure isn't even gaining anything.
They are all wrong because they call generic functions.
@pure functions are only allow to call intrinsics and built-in functions, which can not be overloaded.
Remembding that map, < == are all generic functions. (=== and fieldtypes are not)

Which ones are not necessary

all, they are all simple enough that they will all constant fold already with out this change.

_SA_hvcat_transposed_size might be actually doing something,
but that is also by far the least safe of any of them for multiple reasons (see below).

Should some of them be replaced by @assume_effects? A

No, none of them should be.
If any of them (except maybe _SA_hvcat_transposed_size) are not constant folding it is probably a bug in julia and should be reported to JuliaLang/julia.

Is there something wrong with @pure (::Type{Tuple})(::Size{S}) where {S} = S? Or @pure size_tuple(::Size{S}) where {S} = Tuple{S...}? Or @pure has_eltype(::Type{<:StaticArray{<:Tuple,T}}) where {T} = @isdefined T

The first two are fine. But the last that uses @isdefined technically isn't (macros are kinda generic functions), but in practice that is one of the safest of all places to break this rule because the methods of that macro will never actually be changed or added to. But also that one can be replaced with true as that PR does and so ...
Since they are all trivial, so there is absolutely no need to have the @pure in the first place.

What could go wrong when the usage is wrong, given that this is one of the most widely used Julia package and no issue has been linked to incorrect use of @pure?

So the most common way (not saying only, but at least most common) things go wrong with incorrect @pure usage is that more specific methods get added.
Normally in julia when a more specific method is added, invalidation is triggeredensuring when next used things that need to be recompiling are recompiled.
The way this works is anywhere there is a static dispatch, (including inlining and constant folding) a backedge is tracked to what method being invalidated.
But @pure functions don't promise to participate in this, so they might say have an answer constant folded in, and then when a more specific method is added in something they call which changes the answer then it might not trigger invalidation.
Could this be fixed you ask? Probably, but it hasn't been because they are not for this, they are for specfic cases in the internals that do not need to worry about this stuff. Normal users should use other tool.

To go into detail about why nothing has broken:
Nothing has gone wrong because they almost all all have pretty tightly restricted type signatures, its all concrete types.
except _eltype_or and _SA_hvcat_transposed_size but those are internal.
and because they are (mostly) locked down to concrete types the methods they call are ones that are not being changed or extended for those types (like map on a Tuple).
So basically it is setup to the main things that can break it are very unlikely to.
Would only break (via the most common way) if you added methos for a new type to internal methods, or if you did the worst kinda type piracy (monkey patching something fully concrete), or changed definition for a new type in a way that is not very sensible.
Technically all these things are legal to do in julia -- julia has well defined behavour in the presence of type piracy and monkey patching.
Infact its a generally very useful feature for prototyping bug-fixes or for fixing stuff when you can't get your fix upstreamed, or sometimes even for testing.
But it ceases to be well defines behavour if you have used @pure on a function that calls generic functions.

So yes, this is relatively safe in practice, but it is also gaining nothing.
And it does break some general garentees we have in the language.

_SA_hvcat_transposed_size has the extra problem that it throws, and as explained in a comment in the source code @pure doesn't treat side-effects like throwing right.
This one in theory can get really weird because the compiler when doing various inferency things is allowed to make up values out of thin air (that may be nonsense) to check things as long as there are no side effects which @pure has said there isn't...
in practice i think its never broken for this reason because the compile mostly does that trick with scalars and the input to this function is not a scalar. But it might start doing so at any time.
And so only a problem ir real input to this function violate that rule, which they do not because that check is presumably more like an assertion asserting something the dev knows to be true rather than a user error? Or maybe it is and users just don't make that error often enough for it to occur in a place where @pure causes bad behavour in this.

[mateuszbaran]

Thank you for a very detailed explanation! I will look into removing those uses of @pure.

[mateuszbaran]

I will try to go slowly with removal of @pure. I've removed the first batch in #1253 .

I don't know @isdefined well enough to know if the change proposed in this PR is indeed equivalent to the old code.

[oxinabox]

Its not possible for type variables to be not defined at runtime in julia.
they must be defined, if they are not it is definately a bug in julia

[oscardssmith]

@oxinabox I'm pretty sure that's wrong.

[mateuszbaran]

I keep forgetting how to trigger the case where @isdefined is needed but Julia itself uses the same pattern in a few places, for example here: https://github.com/JuliaLang/julia/blob/25c8128d079a48d2e964d47dd9d5e39a7d3641d6/base/refpointer.jl#L95

[nsajko]

Not sure if it's relevant for StaticArrays, but here's one way to trigger this:

julia> f(::Type{<:Tuple}) = "fallback"
f (generic function with 1 method)

julia> f(::Type{<:Tuple{Vararg{T}}}) where {T} = (@isdefined T) ? "is defined" : "is not defined"
f (generic function with 2 methods)

julia> struct S end

julia> f(Tuple)
"fallback"

julia> f(Tuple{S})
"is defined"

julia> f(Tuple{T} where {T<:S})
"is defined"

julia> f(Tuple{Vararg{T}} where {T<:S})
"is not defined"

Xref JuliaLang/julia#52992