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

Update for v0.7 support #13

Merged
merged 4 commits into from
May 11, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ matrix:
# - julia -e 'Pkg.clone(pwd()); Pkg.build("InternedStrings"); Pkg.test("InternedStrings"; coverage=true)'
after_success:
# push coverage results to Coveralls
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use just CodeCov and not Coveralls?
I don't see the point in having both, and I'm a bit more used to CodeCov

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine, I've just copied that for that last 3 years from other packages!
I'll remove Coveralls.

- julia -e 'cd(Pkg.dir("InternedStrings")); Pkg.add("Coverage"); using Coverage; Coveralls.submit(Coveralls.process_folder())'
# - julia -e 'cd(Pkg.dir("InternedStrings")); Pkg.add("Coverage"); using Coverage; Coveralls.submit(Coveralls.process_folder())'
# push coverage results to Codecov
- julia -e 'cd(Pkg.dir("InternedStrings")); Pkg.add("Coverage"); using Coverage; Codecov.submit(Codecov.process_folder())'
2 changes: 1 addition & 1 deletion src/InternedStrings.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ module InternedStrings
export @i_str, intern

Base.@deprecate_binding(InternedString, String, true)
#InternedString(s)=intern(String(s))

@static if VERSION < v"0.7.0-DEV"
const Nothing = Void
Expand Down Expand Up @@ -98,6 +97,7 @@ end


macro i_str(s)
# This is done to get interpolation to work correctly
true_string_expr = esc(Meta.parse(string('"', unescape_string(s), '"')))
Copy link
Member

@oxinabox oxinabox May 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment is likely wanted here explaining that is is making interpolation happen
Probably something I missed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will add.

Expr(:call, intern, true_string_expr)
end
Expand Down
20 changes: 10 additions & 10 deletions test/all_kinds_of_types.jl
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
@testset "String" begin
ex1 = intern("ex")
@test ex1 == "ex"
V6_COMPAT ? (@test ex1 !== "ex") : (@test_broken !object_id_eq(ex1, "ex"))
@test !addr_eq(ex1, "ex")
ex2 = intern("ex")
@test object_id_eq(ex1, ex2)
@test addr_eq(ex1, ex2)
ex3 = intern(String, "ex")
@test object_id_eq(ex1, ex3)
@test addr_eq(ex1, ex3)

@testset "type inference" begin
@test ex1 isa String
Expand All @@ -21,9 +21,9 @@ end
aa3, bb3, cc3 = intern.(String, split("aa bb cc"))

@test bb1 == "bb"
V6_COMPAT ? (@test bb1 !== "bb") : (@test_broken !object_id_eq(bb1, "bb"))
@test object_id_eq(bb1, bb2)
@test object_id_eq(bb1, bb3)
@test !addr_eq(bb1, "bb")
@test addr_eq(bb1, bb2)
@test addr_eq(bb1, bb3)

@testset "type inference" begin
@test intern(split("aa bb cc")[1]) isa String
Expand All @@ -38,21 +38,21 @@ end
s1 = "ex"
s2 = "ex"
ex1 = @inferred intern(String, WeakRefString(unsafe_wrap(Vector{UInt8}, s1)))
V6_COMPAT ? (@test ex1 !== s1) : (@test_broken !object_id_eq(ex1, s1))
@test !addr_eq(ex1, s1)
@test ex1 isa String
ex2 = @inferred intern(String, WeakRefString(unsafe_wrap(Vector{UInt8}, s2)))
@test object_id_eq(ex1, ex2)
@test addr_eq(ex1, ex2)
end

# Enable when https://github.com/JuliaLang/julia/issues/26939 is fixed
false && @testset "BigFloat" begin
let
pi1 = intern(BigFloat(π))
@test pi1 == BigFloat(π)
@test !object_id_eq(pi1, BigFloat(π))
@test !addr_eq(pi1, BigFloat(π))

pi2 = intern(BigFloat(π))
@test object_id_eq(pi1, pi2)
@test addr_eq(pi1, pi2)

@testset "type inference" begin
@test pi1 isa BigFloat
Expand Down
34 changes: 10 additions & 24 deletions test/corefunctionality.jl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ end
@test s == s
@test s == "Hello My Friends1"

@test object_id_eq(intern(s), s)
@test addr_eq(intern(s), s)
end end


Expand All @@ -30,46 +30,32 @@ end end
empty!(InternedStrings.pool)

# sanity check that strings are not already Interning
V6_COMPAT ? (@test !object_id_eq(a, b)) : (@test_broken !object_id_eq(a, b))
@test !addr_eq(a, b)

ai = intern(a)
bi = intern(b)
@test object_id_eq(ai, bi)
@test addr_eq(ai, bi)

@test intern("a $(2*54) c") == "a 108 c"
end
end

#=
a = "Gold"
typeof(a), objectid(a) # This is the orignal reference
a = intern(a)
typeof(a), objectid(a) # No change still same memory
b = "Gold"
typeof(b),objectid(b) # New memory, see different ID
b = intern(b) # Replace it, now the memory with id= can be freed
typeof(b),objectid(b) # See it is same memory as for the original `a`
objectid(intern("Gold")) # Same again
=#

@testset "ID check" begin
let a = "Gold", b = "Gold"
let a = "Gold", b = String(b"Gold")

empty!(InternedStrings.pool)

target_id = objectid(a)
target_addr = pointer(a)

a = intern(a)
@test objectid(a) == target_id
@test pointer(a) == target_addr

(V6_COMPAT
? (@test objectid(b) != target_id)
: (@test_broken !object_id_eq(objectid(b), target_id)))
@test pointer(b) != target_addr

b = intern(b)
@test objectid(b) == target_id
@test pointer(b) == target_addr

@test objectid(intern("Gold")) == target_id
@test pointer(intern(SubString("Gold", 1))) == target_addr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a different test.
A useful test, but a different one, because interning SubStrings works differently
However it doesn't exactly matter because the test was redundant anyway.
This whole test suite is basically checking the example from the readme works still.

And it is now out of sync with that, but I'll replace it anyway

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't realize that interning SubStrings was handled differently, I was just trying to break the connection between the string literals.


use(a,b)
end
Expand All @@ -89,7 +75,7 @@ end end
@test length(InternedStrings.pool)==0
ai = intern("Hello My Friends4")
bi = intern(join(["Hello", "My", "Friends4"], " "))
@test object_id_eq(ai, bi)
@test addr_eq(ai, bi)
@test length(InternedStrings.pool)==1
use(ai,bi)
ai = [44]
Expand Down
3 changes: 1 addition & 2 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ const V6_COMPAT = VERSION < v"0.7.0-DEV"
using Base.Test
unsafe_wrap(::Type{Vector{UInt8}}, str) = Vector{UInt8}(str)
const objectid = object_id
object_id_eq(a,b) = a === b
else
using Test, Random
object_id_eq(a,b) = objectid(a) == objectid(b)
const gc = GC.gc
end
addr_eq(a,b) = pointer(a) === pointer(b)

@testset "All kinds of types" begin include("all_kinds_of_types.jl") end
@testset "String macro" begin include("string_macro.jl") end
Expand Down
6 changes: 3 additions & 3 deletions test/string_macro.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
@test i"abc" == i"abc"

@test i"a\na\na\na" == join(fill("a", 4), "\n")
@test object_id_eq(i"a\na\na\na", intern(join(fill("a", 4), "\n")))
@test addr_eq(i"a\na\na\na", intern(join(fill("a", 4), "\n")))

@test i"a $(2*54) c" == "a 108 c"
@test object_id_eq(i"a $(2*54) c", i"a 108 c")
@test addr_eq(i"a $(2*54) c", i"a 108 c")

x = "cruel"
@test i"hello $x world" == "hello cruel world"
@test object_id_eq(i"hello $x world", i"hello cruel world")
@test addr_eq(i"hello $x world", i"hello cruel world")
end