Skip to content

Commit

Permalink
Remove some automatic conversion and comparison
Browse files Browse the repository at this point in the history
- Remove methods that allow comparing an object of type `CxxRef{T}` with
  one of type `T`. Note that other `Ref` types in Julia don't provide such
  methods either.
- Remove `convert` methods from `SmartPointer` instances to to other types.

In both cases, the goal is to avoid invalidations. To give an idea of how
bad the removed methods were, consider the number of invalidations as
computed with this snippet in a fresh Julia session:

    using SnoopCompileCore
    invalidations = @Snoopr using CxxWrap;
    length(invalidations)

Before this patch, we get 20819 invalidations with Julia 1.8.4, and
13863 with Julia master.

After this patch, this is down to just 2695 in 1.8.4, respectively
1480 in Julia master.

The main issue with this patch is that it may break code relying on
these convenience methods. However, the resulting failures should be
easy to fix, and the fixed code should work fine with older CxxWrap
versions. I would nevertheless suggest to mark this as a breaking change
by following the appropriate semver rules.
  • Loading branch information
fingolfin committed Dec 29, 2022
1 parent f08ad8a commit 19c9051
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 22 deletions.
6 changes: 0 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -566,12 +566,6 @@ The shared pointer can then be used in a function taking an object of type `Worl

```julia
swf = CppTypes.shared_world_factory()
CppTypes.greet(swf)
```

Explicit dereferencing is also supported, using the `[]` operator:

```julia
CppTypes.greet(swf[])
```

Expand Down
10 changes: 0 additions & 10 deletions src/CxxWrap.jl
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,6 @@ Base.convert(t::Type{<:CxxBaseRef{T}}, x::Ptr{NT}) where {T <: CxxNumber, NT <:
Base.:(==)(a::Union{CxxPtr,ConstCxxPtr}, b::Union{CxxPtr,ConstCxxPtr}) = (a.cpp_object == b.cpp_object)
Base.:(==)(a::CxxBaseRef, b::Ptr) = (a.cpp_object == b)
Base.:(==)(a::Ptr, b::CxxBaseRef) = (b == a)
Base.:(==)(a::Union{CxxRef,ConstCxxRef}, b) = (a[] == b)
Base.:(==)(a, b::Union{CxxRef,ConstCxxRef}) = (b == a)
Base.:(==)(a::Union{CxxRef,ConstCxxRef}, b::Union{CxxRef,ConstCxxRef}) = (a[] == b[])

_deref(p::CxxBaseRef, ::Type) = unsafe_load(p.cpp_object)
_deref(p::CxxBaseRef{T}, ::Type{IsCxxType}) where {T} = dereferenced_type(T)(p.cpp_object)
Expand Down Expand Up @@ -272,13 +269,6 @@ Base.setindex!(x::CxxBaseRef, val, i::Int) = Base.setindex!(x[], val, i)

Base.convert(::Type{RT}, p::SmartPointer{T}) where {T, RT <: CxxBaseRef{T}} = p[]
Base.cconvert(::Type{RT}, p::SmartPointer{T}) where {T, RT <: CxxBaseRef{T}} = p[]
function Base.convert(::Type{T1}, p::SmartPointer) where {T1}
return cxxupcast(T1, p[])[]
end
function Base.convert(to_type::Type{<:Ref{T1}}, p::SmartPointer) where {T1}
return to_type(convert(T1,p))
end
Base.convert(::Type{Any}, x::SmartPointer) = x

Base.unsafe_convert(to_type::Type{<:CxxBaseRef}, x) = to_type(x.cpp_object)

Expand Down
6 changes: 3 additions & 3 deletions test/inheritance.jl
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ d_ptr = shared_d()
@test shared_ptr_message(c_ptr) == "C"
@test shared_ptr_message(d_ptr) == "D"

@test message(b_ptr) == "B"
@test message(c_ptr) == "C"
@test message(d_ptr) == "D"
@test message(b_ptr[]) == "B"
@test message(c_ptr[]) == "C"
@test message(d_ptr[]) == "D"

@test weak_ptr_message_b(b_ptr) == "B"
@test weak_ptr_message_a(b_ptr) == "B"
Expand Down
4 changes: 2 additions & 2 deletions test/stdlib.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ let s = StdString("test")
println("This prints a test string: ", s)
@test s == "test"
stref = CxxRef(s)
@test stref == s
@test stref[] == s
@test stref == stref
end

Expand All @@ -32,7 +32,7 @@ end
let s = StdString("foo")
@test String(s) == "foo"
sref = CxxRef(s)
@test sref == "foo"
@test sref[] == "foo"
@test String(sref) == "foo"
@test unsafe_string(CxxWrap.StdLib.c_str(s)) == "foo"
@test unsafe_string(CxxWrap.StdLib.c_str(s),2) == "fo"
Expand Down
1 change: 0 additions & 1 deletion test/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,6 @@ let weq = CppTypes.World()
weqptr2 = CxxPtr(weq)
@test weqptr1 == weqptr2
@test weqref1 == weqref2
@test weq == weqref1
@test weq == weqref1[]
@test weqref2[] == weqptr1[]
d = Dict{CppTypes.World, Int}()
Expand Down

0 comments on commit 19c9051

Please sign in to comment.