diff --git a/src/builtins.c b/src/builtins.c index 4f75fd79eadcc..81afa8bae482a 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -1694,36 +1694,48 @@ static int equiv_field_types(jl_value_t *old, jl_value_t *ft) // inline it. The only way fields can reference this type (due to // syntax-enforced restrictions) is via being passed as a type parameter. Thus // we can conservatively check this by examining only the parameters of the -// dependent types. -// affects_layout is a hack introduced by #35275 to workaround a problem -// introduced by #34223: it checks whether we will potentially need to -// compute the layout of the object before we have fully computed the types of -// the fields during recursion over the allocation of the parameters for the -// field types (of the concrete subtypes) -static int references_name(jl_value_t *p, jl_typename_t *name, int affects_layout) JL_NOTSAFEPOINT -{ - if (jl_is_uniontype(p)) - return references_name(((jl_uniontype_t*)p)->a, name, affects_layout) || - references_name(((jl_uniontype_t*)p)->b, name, affects_layout); - if (jl_is_unionall(p)) - return references_name((jl_value_t*)((jl_unionall_t*)p)->var->lb, name, 0) || - references_name((jl_value_t*)((jl_unionall_t*)p)->var->ub, name, 0) || - references_name(((jl_unionall_t*)p)->body, name, affects_layout); +// dependent types. Additionally, a field might have already observed this +// object for layout purposes before we got around to deciding if inlining +// would be possible, so we cannot change the layout now if so. +// affects_layout is a (conservative) analysis of layout_uses_free_typevars +// freevars is a (conservative) analysis of what calling jl_has_bound_typevars from name->wrapper gives (TODO: just call this instead?) +static int references_name(jl_value_t *p, jl_typename_t *name, int affects_layout, int freevars) JL_NOTSAFEPOINT +{ + if (freevars && !jl_has_free_typevars(p)) + freevars = 0; + while (jl_is_unionall(p)) { + if (references_name((jl_value_t*)((jl_unionall_t*)p)->var->lb, name, 0, freevars) || + references_name((jl_value_t*)((jl_unionall_t*)p)->var->ub, name, 0, freevars)) + return 1; + p = ((jl_unionall_t*)p)->body; + } + if (jl_is_uniontype(p)) { + return references_name(((jl_uniontype_t*)p)->a, name, affects_layout, freevars) || + references_name(((jl_uniontype_t*)p)->b, name, affects_layout, freevars); + } if (jl_is_typevar(p)) return 0; // already checked by unionall, if applicable if (jl_is_datatype(p)) { jl_datatype_t *dp = (jl_datatype_t*)p; if (affects_layout && dp->name == name) return 1; - // affects_layout checks whether we will need to attempt to layout this - // type (based on whether all copies of it have the same layout) in - // that case, we still need to check the recursive parameters for - // layout recursion happening also, but we know it won't itself cause - // problems for the layout computation affects_layout = ((jl_datatype_t*)jl_unwrap_unionall(dp->name->wrapper))->layout == NULL; + // and even if it has a layout, the fields themselves might trigger layouts if they use tparam i + // rather than checking this for each field, we just assume it applies + if (!affects_layout && freevars && jl_field_names(dp) != jl_emptysvec) { + jl_svec_t *types = ((jl_datatype_t*)jl_unwrap_unionall(dp->name->wrapper))->types; + size_t i, l = jl_svec_len(types); + for (i = 0; i < l; i++) { + jl_value_t *ft = jl_svecref(types, i); + if (!jl_is_typevar(ft) && jl_has_free_typevars(ft)) { + affects_layout = 1; + break; + } + } + } size_t i, l = jl_nparams(p); for (i = 0; i < l; i++) { - if (references_name(jl_tparam(p, i), name, affects_layout)) + if (references_name(jl_tparam(p, i), name, affects_layout, freevars)) return 1; } } @@ -1759,12 +1771,12 @@ JL_CALLABLE(jl_f__typebody) // able to compute the layout of the object before needing to // publish it, so we must assume it cannot be inlined, if that // check passes, then we also still need to check the fields too. - if (!dt->name->mutabl && (nf == 0 || !references_name((jl_value_t*)dt->super, dt->name, 1))) { + if (!dt->name->mutabl && (nf == 0 || !references_name((jl_value_t*)dt->super, dt->name, 0, 1))) { int mayinlinealloc = 1; size_t i; for (i = 0; i < nf; i++) { jl_value_t *fld = jl_svecref(ft, i); - if (references_name(fld, dt->name, 1)) { + if (references_name(fld, dt->name, 1, 1)) { mayinlinealloc = 0; break; } diff --git a/test/core.jl b/test/core.jl index f90a3d968c19b..e65cf08be9e15 100644 --- a/test/core.jl +++ b/test/core.jl @@ -374,8 +374,8 @@ let ft = Base.datatype_fieldtypes @test ft(elT2.body)[1].parameters[1] === elT2 @test Base.isconcretetype(ft(elT2.body)[1]) end -#struct S22624{A,B,C} <: Ref{S22624{Int64,A}}; end -@test_broken @isdefined S22624 +struct S22624{A,B,C} <: Ref{S22624{Int,A}}; end +@test sizeof(S22624) == sizeof(S22624{Int,Int,Int}) == 0 # issue #42297 mutable struct Node42297{T, V} @@ -414,6 +414,18 @@ mutable struct FooFoo{A,B} y::FooFoo{A} end @test FooFoo{Int} <: FooFoo{Int,AbstractString}.types[1] +# make sure this self-referential struct doesn't crash type layout +struct SelfTyA{V} + a::Base.RefValue{V} +end +struct SelfTyB{T} + a::T + b::SelfTyA{SelfTyB{T}} +end +let T = Base.RefValue{SelfTyB{Int}} + @test sizeof(T) === sizeof(Int) + @test sizeof(T.types[1]) === 2 * sizeof(Int) +end let x = (2,3) @test +(x...) == 5 @@ -4110,7 +4122,29 @@ end let z1 = Z14477() @test isa(z1, Z14477) @test isa(z1.fld, Z14477) + @test isdefined(z1, :fld) + @test !isdefined(z1.fld, :fld) +end +struct Z14477B + fld::Union{Nothing,Z14477B} + Z14477B() = new(new(nothing)) end +let z1 = Z14477B() + @test isa(z1, Z14477B) + @test isa(z1.fld, Z14477B) + @test isa(z1.fld.fld, Nothing) +end +struct Z14477C{T} + fld::Z14477C{Int8} + Z14477C() = new{Int16}(new{Int8}()) +end +let z1 = Z14477C() + @test isa(z1, Z14477C) + @test isa(z1.fld, Z14477C) + @test isdefined(z1, :fld) + @test !isdefined(z1.fld, :fld) +end + # issue #8846, generic macros macro m8846(a, b=0)