Skip to content

Commit

Permalink
fix some issues with abstract named tuple types (#32725)
Browse files Browse the repository at this point in the history
fixes #32697, fixes #32698
  • Loading branch information
JeffBezanson authored Aug 5, 2019
1 parent 997ce45 commit 53beb34
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 36 deletions.
20 changes: 18 additions & 2 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -745,8 +745,24 @@ function getfield_tfunc(@nospecialize(s00), @nospecialize(name))
return Any
end
if s.name === _NAMEDTUPLE_NAME && !isconcretetype(s)
# TODO: better approximate inference
return Any
if isa(name, Const) && isa(name.val, Symbol)
if isa(s.parameters[1], Tuple)
name = Const(Int(ccall(:jl_field_index, Cint, (Any, Any, Cint), s, name.val, false)+1))
else
name = Int
end
elseif Symbol name
name = Int
end
_ts = s.parameters[2]
while isa(_ts, TypeVar)
_ts = _ts.ub
end
_ts = rewrap_unionall(_ts, s00)
if !(_ts <: Tuple)
return Any
end
return getfield_tfunc(_ts, name)
end
ftypes = datatype_fieldtypes(s)
if isempty(ftypes)
Expand Down
67 changes: 36 additions & 31 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -799,44 +799,49 @@ static jl_value_t *get_fieldtype(jl_value_t *t, jl_value_t *f, int dothrow)
int field_index;
if (jl_is_long(f)) {
field_index = jl_unbox_long(f) - 1;
if (st->name == jl_namedtuple_typename) {
jl_value_t *nm = jl_tparam0(st);
if (jl_is_tuple(nm)) {
int nf = jl_nfields(nm);
if (field_index < 0 || field_index >= nf) {
if (dothrow)
jl_bounds_error(t, f);
else
return jl_bottom_type;
}
}
jl_value_t *tt = jl_tparam1(st);
while (jl_is_typevar(tt))
tt = ((jl_tvar_t*)tt)->ub;
if (tt == (jl_value_t*)jl_any_type)
return (jl_value_t*)jl_any_type;
return get_fieldtype(tt, f, dothrow);
}
jl_svec_t *types = jl_get_fieldtypes(st);
int nf = jl_svec_len(types);
if (nf > 0 && field_index >= nf-1 && st->name == jl_tuple_typename) {
jl_value_t *ft = jl_field_type(st, nf-1);
if (jl_is_vararg_type(ft))
return jl_unwrap_vararg(ft);
}
if (field_index < 0 || field_index >= nf) {
if (dothrow)
jl_bounds_error(t, f);
else
return jl_bottom_type;
}
}
else {
JL_TYPECHK(fieldtype, symbol, f);
field_index = jl_field_index(st, (jl_sym_t*)f, dothrow);
if (field_index == -1)
return jl_bottom_type;
}
if (st->name == jl_namedtuple_typename) {
jl_value_t *nm = jl_tparam0(st);
if (jl_is_tuple(nm)) {
int nf = jl_nfields(nm);
if (field_index < 0 || field_index >= nf) {
if (dothrow)
jl_bounds_error(t, f);
else
return jl_bottom_type;
}
}
jl_value_t *tt = jl_tparam1(st);
while (jl_is_typevar(tt))
tt = ((jl_tvar_t*)tt)->ub;
if (tt == (jl_value_t*)jl_any_type)
return (jl_value_t*)jl_any_type;
JL_GC_PUSH1(&f);
if (jl_is_symbol(f))
f = jl_box_long(field_index+1);
jl_value_t *ft = get_fieldtype(tt, f, dothrow);
JL_GC_POP();
return ft;
}
jl_svec_t *types = jl_get_fieldtypes(st);
int nf = jl_svec_len(types);
if (nf > 0 && field_index >= nf-1 && st->name == jl_tuple_typename) {
jl_value_t *ft = jl_field_type(st, nf-1);
if (jl_is_vararg_type(ft))
return jl_unwrap_vararg(ft);
}
if (field_index < 0 || field_index >= nf) {
if (dothrow)
jl_bounds_error(t, f);
else
return jl_bottom_type;
}
return jl_field_type(st, field_index);
}

Expand Down
22 changes: 19 additions & 3 deletions src/datatype.c
Original file line number Diff line number Diff line change
Expand Up @@ -905,9 +905,25 @@ JL_DLLEXPORT jl_value_t *jl_new_struct_uninit(jl_datatype_t *type)
JL_DLLEXPORT int jl_field_index(jl_datatype_t *t, jl_sym_t *fld, int err)
{
jl_svec_t *fn = jl_field_names(t);
for(size_t i=0; i < jl_svec_len(fn); i++) {
if (jl_svecref(fn,i) == (jl_value_t*)fld) {
return (int)i;
size_t n = jl_svec_len(fn);
if (n == 0) {
if (jl_is_namedtuple_type(t)) {
jl_value_t *ns = jl_tparam0(t);
if (jl_is_tuple(ns)) {
n = jl_nfields(ns);
for(size_t i=0; i < n; i++) {
if (jl_get_nth_field(ns, i) == (jl_value_t*)fld) {
return (int)i;
}
}
}
}
}
else {
for(size_t i=0; i < n; i++) {
if (jl_svecref(fn,i) == (jl_value_t*)fld) {
return (int)i;
}
}
}
if (err)
Expand Down
15 changes: 15 additions & 0 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1460,6 +1460,21 @@ f_pure_add() = (1 + 1 == 2) ? true : "FAIL"
@test Core.Compiler.getfield_tfunc(Const(Vector{Int}), Const(:mutable)) == Const(true)
@test Core.Compiler.getfield_tfunc(DataType, Const(:mutable)) == Bool

# getfield on abstract named tuples. issue #32698
import Core.Compiler.getfield_tfunc
@test getfield_tfunc(NamedTuple{(:id, :y), T} where {T <: Tuple{Int, Union{Float64, Missing}}},
Const(:y)) == Union{Missing, Float64}
@test getfield_tfunc(NamedTuple{(:id, :y), T} where {T <: Tuple{Int, Union{Float64, Missing}}},
Const(2)) == Union{Missing, Float64}
@test getfield_tfunc(NamedTuple{(:id, :y), T} where {T <: Tuple{Int, Union{Float64, Missing}}},
Symbol) == Union{Missing, Float64, Int}
@test getfield_tfunc(NamedTuple{<:Any, T} where {T <: Tuple{Int, Union{Float64, Missing}}},
Symbol) == Union{Missing, Float64, Int}
@test getfield_tfunc(NamedTuple{<:Any, T} where {T <: Tuple{Int, Union{Float64, Missing}}},
Int) == Union{Missing, Float64, Int}
@test getfield_tfunc(NamedTuple{<:Any, T} where {T <: Tuple{Int, Union{Float64, Missing}}},
Const(:x)) == Union{Missing, Float64, Int}

struct Foo_22708
x::Ptr{Foo_22708}
end
Expand Down
4 changes: 4 additions & 0 deletions test/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,10 @@ tlayout = TLayout(5,7,11)
@test fieldtype((NamedTuple{(:a,:b),T} where T<:Tuple{Vararg{Integer}}), 2) === Integer
@test_throws BoundsError fieldtype(NamedTuple{(:a,:b)}, 3)

# issue #32697
@test fieldtype(NamedTuple{(:x,:y), T} where T <: Tuple{Int, Union{Float64, Missing}}, :x) == Int
@test fieldtype(NamedTuple{(:x,:y), T} where T <: Tuple{Int, Union{Float64, Missing}}, :y) == Union{Float64, Missing}

@test fieldtypes(NamedTuple{(:a,:b)}) == (Any, Any)
@test fieldtypes((NamedTuple{T,Tuple{Int,String}} where T)) === (Int, String)
@test fieldtypes(TLayout) === (Int8, Int16, Int32)
Expand Down

0 comments on commit 53beb34

Please sign in to comment.