Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make empty Size() type stable #28

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

jipolanco
Copy link
Contributor

The Size() constructor (used with no arguments) is currently type unstable, at least on Julia 1.10:

julia> @code_warntype Size()
MethodInstance for Size()
  from Size(s::Union{StaticArraysCore.Dynamic, Int64}...) @ StaticArraysCore ~/.julia/packages/StaticArraysCore/P6PH7/src/StaticArraysCore.jl:476
Arguments
  #self#::Type{Size}
  s::Tuple{}
Body::Size   # <-- in red
1nothing%2 = Core.apply_type(StaticArraysCore.Size, s)::Type{Size{_A}} where _A   # <-- in red%3 = (%2)()::Size   # <-- in red
└──      return %3

This doesn't seem to be related to the @pure annotations in the Size constructors.

This PR fixes this issue by explicitly creating a Size() constructor which is type stable.


For context, this seems to fix an obscure type stability issue I identified when broadcasting a Number and a SVector, which only manifests when Julia is launched with --check-bounds=no. This looks similar to JuliaArrays/StaticArrays.jl#1155, but I'm not sure it's exactly the same issue.

As an example:

using StaticArrays
x = 0.3
y = SVector(0.1, 0.2, 0.3)
@code_warntype x .+ y

On Julia 1.10.4 started with default flags everything looks fine:

julia> @code_warntype x .+ y
MethodInstance for (::var"##dotfunction#225#1")(::Float64, ::SVector{3, Float64})
  from (::var"##dotfunction#225#1")(x1, x2) @ Main none:0
Arguments
  #self#::Core.Const(var"##dotfunction#225#1"())
  x1::Float64
  x2::SVector{3, Float64}
Body::SVector{3, Float64}
1%1 = Base.broadcasted(Main.:+, x1, x2)::Base.Broadcast.Broadcasted{StaticArraysCore.StaticArrayStyle{1}, Nothing, typeof(+), Tuple{Float64, SVector{3, Float64}}}%2 = Base.materialize(%1)::SVector{3, Float64}
└──      return %2

When Julia is launched with --check-bounds=no:

julia> @code_warntype x .+ y
MethodInstance for (::var"##dotfunction#225#1")(::Float64, ::SVector{3, Float64})
  from (::var"##dotfunction#225#1")(x1, x2) @ Main none:0
Arguments
  #self#::Core.Const(var"##dotfunction#225#1"())
  x1::Float64
  x2::SVector{3, Float64}
Body::Any
1%1 = Base.broadcasted(Main.:+, x1, x2)::Base.Broadcast.Broadcasted{StaticArraysCore.StaticArrayStyle{1}, Nothing, typeof(+), Tuple{Float64, SVector{3, Float64}}}%2 = Base.materialize(%1)::Any
└──      return %2

The culprit can be tracked down to be the type unstable Size() constructor. Note that Size() is also unstable without --check-bounds=no, but in that case Julia is somehow able to determine the correct return type of the broadcast.

@mateuszbaran
Copy link
Collaborator

Thanks, LGTM. Could you just bump patch version?

@jipolanco
Copy link
Contributor Author

Done!

@mateuszbaran mateuszbaran merged commit ec3ce1c into JuliaArrays:main Jun 7, 2024
16 checks passed
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