From 89bec0a439720b9cbb279594950ec613bc5f3ce5 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Thu, 27 Feb 2025 21:15:17 +0900 Subject: [PATCH 1/3] inference: fix the ptrfree field check The previous implementation using `is_undefref_field` was flawed. We can now implement a more robust check with `DataTypeFieldDesc`. --- Compiler/src/abstractinterpretation.jl | 14 +++++++++++--- Compiler/src/tfuncs.jl | 12 ------------ Compiler/test/effects.jl | 3 +++ 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl index 1728965e3702c..9077a02c5d063 100644 --- a/Compiler/src/abstractinterpretation.jl +++ b/Compiler/src/abstractinterpretation.jl @@ -3079,6 +3079,15 @@ function abstract_eval_call(interp::AbstractInterpreter, e::Expr, sstate::Statem end end +# TODO: try doing layout of dt so this doesn’t crash julia +# function is_field_pointerfree(dt::DataType, fidx::Int) +# DataTypeFieldDesc(dt)[fidx].isptr && return false +# ft = fieldtype(dt, fidx) +# return ft isa DataType && datatype_pointerfree(ft) +# end +maybe_field_pointerfree(@nospecialize ft) = + !isconcretetype(ft) || datatype_pointerfree(ft) + function abstract_eval_new(interp::AbstractInterpreter, e::Expr, sstate::StatementState, sv::AbsIntState) 𝕃ᵢ = typeinf_lattice(interp) @@ -3089,9 +3098,8 @@ function abstract_eval_new(interp::AbstractInterpreter, e::Expr, sstate::Stateme ismutable = ismutabletype(ut) fcount = datatype_fieldcount(ut) nargs = length(e.args) - 1 - has_any_uninitialized = (fcount === nothing || (fcount > nargs && (let t = rt - any(i::Int -> !is_undefref_fieldtype(fieldtype(t, i)), (nargs+1):fcount) - end))) + has_any_uninitialized = fcount === nothing || (fcount > nargs && + any(i::Int->maybe_field_pointerfree(fieldtype(ut,i)), (nargs+1):fcount)) if has_any_uninitialized # allocation with undefined field is inconsistent always consistent = ALWAYS_FALSE diff --git a/Compiler/src/tfuncs.jl b/Compiler/src/tfuncs.jl index 98c062ff5b6ca..3c23974c88920 100644 --- a/Compiler/src/tfuncs.jl +++ b/Compiler/src/tfuncs.jl @@ -1286,18 +1286,6 @@ end return rewrap_unionall(R, s00) end -# checks if a field of this type is guaranteed to be defined to a value -# and that access to an uninitialized field will cause an `UndefRefError` or return zero -# - is_undefref_fieldtype(String) === true -# - is_undefref_fieldtype(Integer) === true -# - is_undefref_fieldtype(Any) === true -# - is_undefref_fieldtype(Int) === false -# - is_undefref_fieldtype(Union{Int32,Int64}) === false -# - is_undefref_fieldtype(T) === false -function is_undefref_fieldtype(@nospecialize ftyp) - return !has_free_typevars(ftyp) && !allocatedinline(ftyp) -end - @nospecs function setfield!_tfunc(𝕃::AbstractLattice, o, f, v, order) if !isvarargtype(order) hasintersect(widenconst(order), Symbol) || return Bottom diff --git a/Compiler/test/effects.jl b/Compiler/test/effects.jl index cbb77e1d8e2a3..fa97c0c94c58d 100644 --- a/Compiler/test/effects.jl +++ b/Compiler/test/effects.jl @@ -266,6 +266,9 @@ end |> Compiler.is_consistent @test Base.infer_effects() do Maybe{String}()[] end |> Compiler.is_consistent +@test Base.infer_effects() do + Maybe{Some{Base.RefValue{Int}}}() +end |> Compiler.is_consistent let f() = Maybe{String}()[] @test Base.return_types() do f() # this call should be concrete evaluated From 6aeb4e572be2654d3673c3f0f92acd2b1b5564ec Mon Sep 17 00:00:00 2001 From: Mason Protter Date: Sun, 23 Nov 2025 16:08:43 +0100 Subject: [PATCH 2/3] properly deal with `DataTypes` which have no layout --- Compiler/src/abstractinterpretation.jl | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl index 9077a02c5d063..fc58fc3dfe6af 100644 --- a/Compiler/src/abstractinterpretation.jl +++ b/Compiler/src/abstractinterpretation.jl @@ -3079,14 +3079,12 @@ function abstract_eval_call(interp::AbstractInterpreter, e::Expr, sstate::Statem end end -# TODO: try doing layout of dt so this doesn’t crash julia -# function is_field_pointerfree(dt::DataType, fidx::Int) -# DataTypeFieldDesc(dt)[fidx].isptr && return false -# ft = fieldtype(dt, fidx) -# return ft isa DataType && datatype_pointerfree(ft) -# end -maybe_field_pointerfree(@nospecialize ft) = - !isconcretetype(ft) || datatype_pointerfree(ft) +function is_field_pointerfree(dt::DataType, fidx::Int) + dt.layout == C_NULL && return false + DataTypeFieldDesc(dt)[fidx].isptr && return false + ft = fieldtype(dt, fidx) + return ft isa DataType && datatype_pointerfree(ft) +end function abstract_eval_new(interp::AbstractInterpreter, e::Expr, sstate::StatementState, sv::AbsIntState) @@ -3099,7 +3097,7 @@ function abstract_eval_new(interp::AbstractInterpreter, e::Expr, sstate::Stateme fcount = datatype_fieldcount(ut) nargs = length(e.args) - 1 has_any_uninitialized = fcount === nothing || (fcount > nargs && - any(i::Int->maybe_field_pointerfree(fieldtype(ut,i)), (nargs+1):fcount)) + any(i::Int->is_field_pointerfree(ut, i), (nargs+1):fcount)) if has_any_uninitialized # allocation with undefined field is inconsistent always consistent = ALWAYS_FALSE From 08c3853539a6ee69cb0ff32c85c19fad3c6f7b9f Mon Sep 17 00:00:00 2001 From: Mason Protter Date: Sun, 23 Nov 2025 20:33:35 +0100 Subject: [PATCH 3/3] Update Compiler/src/abstractinterpretation.jl Co-authored-by: James Wrigley --- Compiler/src/abstractinterpretation.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl index fc58fc3dfe6af..3808797275e15 100644 --- a/Compiler/src/abstractinterpretation.jl +++ b/Compiler/src/abstractinterpretation.jl @@ -3080,7 +3080,7 @@ function abstract_eval_call(interp::AbstractInterpreter, e::Expr, sstate::Statem end function is_field_pointerfree(dt::DataType, fidx::Int) - dt.layout == C_NULL && return false + dt.layout::Ptr{Cvoid} == C_NULL && return false DataTypeFieldDesc(dt)[fidx].isptr && return false ft = fieldtype(dt, fidx) return ft isa DataType && datatype_pointerfree(ft)