Skip to content

Commit

Permalink
Merge pull request #18510 from JuliaLang/nl/hasvalue
Browse files Browse the repository at this point in the history 
Change isnull field of Nullable into hasvalue
  • Loading branch information
@nalimilan
nalimilan authored Sep 29, 2016
2 parents 54f3c90 + 32a08c7 commit b36141f
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 49 deletions.
6 changes: 3 additions & 3 deletions base/base.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ gc(full::Bool=true) = ccall(:jl_gc_collect, Void, (Cint,), full)
gc_enable(on::Bool) = ccall(:jl_gc_enable, Cint, (Cint,), on)!=0

immutable Nullable{T}
isnull::Bool
hasvalue::Bool
value::T

Nullable() = new(true)
Nullable(value::T, isnull::Bool=false) = new(isnull, value)
Nullable() = new(false)
Nullable(value::T, hasvalue::Bool=true) = new(hasvalue, value)
end
27 changes: 13 additions & 14 deletions base/nullable.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
immutable NullException <: Exception
end

Nullable{T}(value::T, isnull::Bool=false) = Nullable{T}(value, isnull)
Nullable{T}(value::T, hasvalue::Bool=true) = Nullable{T}(value, hasvalue)
Nullable() = Nullable{Union{}}()

eltype{T}(::Type{Nullable{T}}) = T
Expand Down Expand Up @@ -53,16 +53,15 @@ otherwise, returns `y` if provided, or throws a `NullException` if not.
"""
@inline function get{S,T}(x::Nullable{S}, y::T)
if isbits(S)
ifelse(x.isnull, y, x.value)
ifelse(isnull(x), y, x.value)
else
x.isnull ? y : x.value
isnull(x) ? y : x.value
end
end

get(x::Nullable) = x.isnull ? throw(NullException()) : x.value

isnull(x::Nullable) = x.isnull
get(x::Nullable) = isnull(x) ? throw(NullException()) : x.value

isnull(x::Nullable) = !x.hasvalue

## Operators

Expand Down Expand Up @@ -107,15 +106,15 @@ and `false` if one is null but not the other: nulls are considered equal.
"""
@inline function isequal{S,T}(x::Nullable{S}, y::Nullable{T})
if null_safe_op(isequal, S, T)
(x.isnull & y.isnull) | (!x.isnull & !y.isnull & isequal(x.value, y.value))
(isnull(x) & isnull(y)) | (!isnull(x) & !isnull(y) & isequal(x.value, y.value))
else
(x.isnull & y.isnull) || (!x.isnull & !y.isnull && isequal(x.value, y.value))
(isnull(x) & isnull(y)) || (!isnull(x) & !isnull(y) && isequal(x.value, y.value))
end
end

isequal(x::Nullable{Union{}}, y::Nullable{Union{}}) = true
isequal(x::Nullable{Union{}}, y::Nullable) = y.isnull
isequal(x::Nullable, y::Nullable{Union{}}) = x.isnull
isequal(x::Nullable{Union{}}, y::Nullable) = isnull(y)
isequal(x::Nullable, y::Nullable{Union{}}) = isnull(x)

null_safe_op{S<:NullSafeTypes,
T<:NullSafeTypes}(::typeof(isless), ::Type{S}, ::Type{T}) = true
Expand All @@ -135,22 +134,22 @@ another null.
@inline function isless{S,T}(x::Nullable{S}, y::Nullable{T})
# NULL values are sorted last
if null_safe_op(isless, S, T)
(!x.isnull & y.isnull) | (!x.isnull & !y.isnull & isless(x.value, y.value))
(!isnull(x) & isnull(y)) | (!isnull(x) & !isnull(y) & isless(x.value, y.value))
else
(!x.isnull & y.isnull) || (!x.isnull & !y.isnull && isless(x.value, y.value))
(!isnull(x) & isnull(y)) || (!isnull(x) & !isnull(y) && isless(x.value, y.value))
end
end

isless(x::Nullable{Union{}}, y::Nullable{Union{}}) = false
isless(x::Nullable{Union{}}, y::Nullable) = false
isless(x::Nullable, y::Nullable{Union{}}) = !x.isnull
isless(x::Nullable, y::Nullable{Union{}}) = !isnull(x)

==(x::Nullable, y::Nullable) = throw(NullException())

const nullablehash_seed = UInt === UInt64 ? 0x932e0143e51d0171 : 0xe51d0171

function hash(x::Nullable, h::UInt)
if x.isnull
if isnull(x)
return h + nullablehash_seed
else
return hash(x.value, h + nullablehash_seed)
Expand Down
24 changes: 12 additions & 12 deletions src/builtins.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ JL_DLLEXPORT jl_nullable_float64_t jl_try_substrtod(char *str, size_t offset, si
char *bstr = str+offset;
char *pend = bstr+len;
char *tofree = NULL;
int err = 0;
int hasvalue = 0;

errno = 0;
if (!(*pend == '\0' || isspace((unsigned char)*pend) || *pend == ',')) {
Expand All @@ -827,28 +827,28 @@ JL_DLLEXPORT jl_nullable_float64_t jl_try_substrtod(char *str, size_t offset, si
double out = jl_strtod_c(bstr, &p);

if (errno==ERANGE && (out==0 || out==HUGE_VAL || out==-HUGE_VAL)) {
err = 1;
hasvalue = 0;
}
else if (p == bstr) {
err = 1;
hasvalue = 0;
}
else {
// Deal with case where the substring might be something like "1 ",
// which is OK, and "1 X", which we don't allow.
err = substr_isspace(p, pend) ? 0 : 1;
hasvalue = substr_isspace(p, pend) ? 1 : 0;
}

if (__unlikely(tofree))
free(tofree);

jl_nullable_float64_t ret = {(uint8_t)err, out};
jl_nullable_float64_t ret = {(uint8_t)hasvalue, out};
return ret;
}

JL_DLLEXPORT int jl_substrtod(char *str, size_t offset, size_t len, double *out)
{
jl_nullable_float64_t nd = jl_try_substrtod(str, offset, len);
if (0 == nd.isnull) {
if (0 != nd.hasvalue) {
*out = nd.value;
return 0;
}
Expand All @@ -866,7 +866,7 @@ JL_DLLEXPORT jl_nullable_float32_t jl_try_substrtof(char *str, size_t offset, si
char *bstr = str+offset;
char *pend = bstr+len;
char *tofree = NULL;
int err = 0;
int hasvalue = 0;

errno = 0;
if (!(*pend == '\0' || isspace((unsigned char)*pend) || *pend == ',')) {
Expand All @@ -890,28 +890,28 @@ JL_DLLEXPORT jl_nullable_float32_t jl_try_substrtof(char *str, size_t offset, si
#endif

if (errno==ERANGE && (out==0 || out==HUGE_VALF || out==-HUGE_VALF)) {
err = 1;
hasvalue = 0;
}
else if (p == bstr) {
err = 1;
hasvalue = 0;
}
else {
// Deal with case where the substring might be something like "1 ",
// which is OK, and "1 X", which we don't allow.
err = substr_isspace(p, pend) ? 0 : 1;
hasvalue = substr_isspace(p, pend) ? 1 : 0;
}

if (__unlikely(tofree))
free(tofree);

jl_nullable_float32_t ret = {(uint8_t)err, out};
jl_nullable_float32_t ret = {(uint8_t)hasvalue, out};
return ret;
}

JL_DLLEXPORT int jl_substrtof(char *str, int offset, size_t len, float *out)
{
jl_nullable_float32_t nf = jl_try_substrtof(str, offset, len);
if (0 == nf.isnull) {
if (0 != nf.hasvalue) {
*out = nf.value;
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions src/julia.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1738,12 +1738,12 @@ JL_DLLEXPORT const char *jl_git_commit(void);

// nullable struct representations
typedef struct {
uint8_t isnull;
uint8_t hasvalue;
double value;
} jl_nullable_float64_t;

typedef struct {
uint8_t isnull;
uint8_t hasvalue;
float value;
} jl_nullable_float32_t;

Expand Down
4 changes: 2 additions & 2 deletions test/core.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3126,13 +3126,13 @@ f11858(Any[Type{Foo11858}, Type{Bar11858}, typeof(g11858)])
foo11904(x::Int) = x
@inline function foo11904{S}(x::Nullable{S})
if isbits(S)
Nullable(foo11904(x.value), x.isnull)
Nullable(foo11904(x.value), x.hasvalue)
else
throw_error()
end
end

@test !foo11904(Nullable(1)).isnull
@test !isnull(foo11904(Nullable(1)))

# issue 11874
immutable Foo11874
Expand Down
32 changes: 16 additions & 16 deletions test/nullable.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ types = [
# Nullable{T}() = new(true)
for T in types
x = Nullable{T}()
@test x.isnull === true
@test x.hasvalue === false
@test isa(x.value, T)
@test eltype(Nullable{T}) === T
@test eltype(x) === T
Expand All @@ -28,28 +28,28 @@ end
# Nullable{T}(value::T) = new(false, value)
for T in types
x = Nullable{T}(zero(T))
@test x.isnull === false
@test x.hasvalue === true
@test isa(x.value, T)
@test x.value === zero(T)
@test eltype(x) === T

x = Nullable{T}(one(T))
@test x.isnull === false
@test x.hasvalue === true
@test isa(x.value, T)
@test x.value === one(T)
@test eltype(x) === T
end

# Nullable{T}(value::T, isnull::Bool) = new(isnull, value)
# Nullable{T}(value::T, hasvalue::Bool) = new(hasvalue, value)
for T in types
x = Nullable{T}(zero(T),false)
@test x.isnull === false
x = Nullable{T}(zero(T), true)
@test x.hasvalue === true
@test isa(x.value, T)
@test x.value === zero(T)
@test eltype(x) === T

x = Nullable{T}(zero(T),true)
@test x.isnull === true
x = Nullable{T}(zero(T), false)
@test x.hasvalue === false
@test isa(x.value, T)
@test eltype(Nullable{T}) === T
@test eltype(x) === T
Expand All @@ -64,13 +64,13 @@ end
for T in types
v = zero(T)
x = Nullable(v)
@test x.isnull === false
@test x.hasvalue === true
@test isa(x.value, T)
@test x.value === v

v = one(T)
x = Nullable(v)
@test x.isnull === false
@test x.hasvalue === true
@test isa(x.value, T)
@test x.value === v
end
Expand Down Expand Up @@ -276,9 +276,9 @@ for S in TestTypes, T in TestTypes
@test isequal(Nullable(u), Nullable(u)) === true
@test isequal(Nullable(v), Nullable(v)) === true

@test isequal(Nullable(u), Nullable(v, true)) === false
@test isequal(Nullable(u, true), Nullable(v)) === false
@test isequal(Nullable(u, true), Nullable(v, true)) === true
@test isequal(Nullable(u), Nullable(v, false)) === false
@test isequal(Nullable(u, false), Nullable(v)) === false
@test isequal(Nullable(u, false), Nullable(v, false)) === true

@test isequal(Nullable(u), Nullable{T}()) === false
@test isequal(Nullable{S}(), Nullable(v)) === false
Expand All @@ -296,9 +296,9 @@ for S in TestTypes, T in TestTypes
@test isless(Nullable(u), Nullable(u)) === false
@test isless(Nullable(v), Nullable(v)) === false

@test isless(Nullable(u), Nullable(v, true)) === true
@test isless(Nullable(u, true), Nullable(v)) === false
@test isless(Nullable(u, true), Nullable(v, true)) === false
@test isless(Nullable(u), Nullable(v, false)) === true
@test isless(Nullable(u, false), Nullable(v)) === false
@test isless(Nullable(u, false), Nullable(v, false)) === false

@test isless(Nullable(u), Nullable{T}()) === true
@test isless(Nullable{S}(), Nullable(v)) === false
Expand Down

2 comments on commit b36141f

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

Please sign in to comment.