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

Inference failure for Ref{Int}? #38133

Open
kcajf opened this issue Oct 21, 2020 · 7 comments
Open

Inference failure for Ref{Int}? #38133

kcajf opened this issue Oct 21, 2020 · 7 comments
Labels
domain:docs This change adds or pertains to documentation

Comments

@kcajf
Copy link
Contributor

kcajf commented Oct 21, 2020

julia> struct Foo
           i::Ref{Int}
       end

julia> f = Foo(Ref(0))
Foo(Base.RefValue{Int64}(0))

julia> g(x) = x.i[]
g (generic function with 1 method)

julia> @code_warntype g(f)
Variables
  #self#::Core.Const(g)
  x::Foo

Body::Any
1 ─ %1 = Base.getproperty(x, :i)::Ref{Int64}
│   %2 = Base.getindex(%1)::Any
└──      return %2

julia> versioninfo()
Julia Version 1.6.0-DEV.1227
Commit 86bbe0941c (2020-10-15 00:29 UTC)

I thought this would infer the return type as Int, but it only manages Any. Am I doing something wrong, or is this expected? Is there a situation where the dereference can return something other than Int?

@kcajf kcajf changed the title Inference failure for Refs Inference failure for Ref{Int}? Oct 21, 2020
@vchuravy
Copy link
Sponsor Member

The issue is that Ref is an abstract type. The concrete type is RefValue.

julia> struct Foo{R<:Ref}
           i::R
       end

julia> g(x) = x.i[]
g (generic function with 1 method)

julia> f = Foo(Ref(0))
Foo{Base.RefValue{Int64}}(Base.RefValue{Int64}(0))

julia> @code_warntype g(f)
Variables
  #self#::Core.Compiler.Const(g, false)
  x::Foo{Base.RefValue{Int64}}

Body::Int64
1 ─ %1 = Base.getproperty(x, :i)::Base.RefValue{Int64}
│   %2 = Base.getindex(%1)::Int64
└──      return %2

So certainly a gotcha, I wouldn't classify it as an inference failure.

@yuyichao yuyichao added the domain:docs This change adds or pertains to documentation label Oct 21, 2020
@kcajf
Copy link
Contributor Author

kcajf commented Oct 22, 2020

Thanks @vchuravy , good to know! I'll use RefValue.

@martinholters
Copy link
Member

Yes, generally better to use Base.RefValue here, but his particular case would also be helped by the following change:

--- a/base/refpointer.jl
+++ b/base/refpointer.jl
@@ -178,7 +178,7 @@ unsafe_convert(::Type{Ptr{T}}, r::Ptr{NTuple{N,T}}) where {N,T} =
 
 ###
 
-getindex(b::RefArray) = b.x[b.i]
+getindex(b::RefArray{T}) where {T} = b.x[b.i]::T
 setindex!(b::RefArray, x) = (b.x[b.i] = x; b)
 
 ###

Note that eltype(Ref{Int}) also promises an Int, so might be worth type-asserting this here anyway. (Or else provide an eltype(Type{<:RefArray}) that delegates to the eltype of the backing array.)

@mchitre
Copy link

mchitre commented Feb 26, 2023

RefValue isn't exported. Better to use it or better to parametrize the struct?

Parametrization feels like an awkward hack as I don't need the struct with any other type than the one declared, and with multiple Ref fields can get messy.

@KristofferC
Copy link
Sponsor Member

Maybe make the struct mutable and const annotate the rest of the fields?

@mchitre
Copy link

mchitre commented Feb 26, 2023

Any cost to a mutable struct with all constant fields as compared to an immutable struct?

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Mar 1, 2023

They have somewhat different semantics: the identity of two identical mutable structs must remain separate (and more importantly coherent), so it's harder for the compiler to completely eliminate allocations. The most straightforward and default way to ensure that is to actually allocate objects with location-based identity.

mutable struct M
    const f::Int
end

struct I
    f::Int
end
# basic identity
julia> I(1) === I(1)
true

julia> M(1) === M(1)
false

# more complex example for I
julia> a = I(1)
I(1)

julia> b = I(1)
I(1)

julia> c = a
I(1)

julia> a === b
true

julia> a === c
true

julia> b === c
true

# more complex example for M
julia> a = M(1)
M(1)

julia> b = M(1)
M(1)

julia> c = a
M(1)

julia> a === b
false

julia> a === c
true

julia> b === c
false

The language needs to ensure that for type M the values a and b are not the same object, whereas a and c are the same object. For the type I there's no such consideration since any instance of I with the same value is the same object, so we don't need to keep track of different ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:docs This change adds or pertains to documentation
Projects
None yet
Development

No branches or pull requests

7 participants