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

effects: handle va-method properly when refining :inaccessiblememonly #48854

Merged
merged 1 commit into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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 base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2823,7 +2823,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
@assert !frame.inferred
frame.dont_work_on_me = true # mark that this function is currently on the stack
W = frame.ip
nargs = narguments(frame)
nargs = narguments(frame, #=include_va=#false)
slottypes = frame.slottypes
ssavaluetypes = frame.ssavaluetypes
bbs = frame.cfg.blocks
Expand Down
8 changes: 5 additions & 3 deletions base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -607,9 +607,11 @@ get_curr_ssaflag(sv::InferenceState) = sv.src.ssaflags[sv.currpc]
add_curr_ssaflag!(sv::InferenceState, flag::UInt8) = sv.src.ssaflags[sv.currpc] |= flag
sub_curr_ssaflag!(sv::InferenceState, flag::UInt8) = sv.src.ssaflags[sv.currpc] &= ~flag

function narguments(sv::InferenceState)
function narguments(sv::InferenceState, include_va::Bool=true)
def = sv.linfo.def
isva = isa(def, Method) && def.isva
nargs = length(sv.result.argtypes) - isva
nargs = length(sv.result.argtypes)
if !include_va
nargs -= isa(def, Method) && def.isva
end
return nargs
end
2 changes: 1 addition & 1 deletion base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ function adjust_effects(sv::InferenceState)
# always throwing an error counts or never returning both count as consistent
ipo_effects = Effects(ipo_effects; consistent=ALWAYS_TRUE)
end
if is_inaccessiblemem_or_argmemonly(ipo_effects) && all(1:narguments(sv)) do i::Int
if is_inaccessiblemem_or_argmemonly(ipo_effects) && all(1:narguments(sv, #=include_va=#true)) do i::Int
return is_mutation_free_argtype(sv.slottypes[i])
end
ipo_effects = Effects(ipo_effects; inaccessiblememonly=ALWAYS_TRUE)
Expand Down
15 changes: 15 additions & 0 deletions test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,21 @@ global inconsistent_condition_ref = Ref{Bool}(false)
end
end |> !Core.Compiler.is_consistent

# should handle va-method properly
callgetfield1(xs...) = getfield(getfield(xs, 1), 1)
@test !Core.Compiler.is_inaccessiblememonly(Base.infer_effects(callgetfield1, (Base.RefValue{Symbol},)))
const GLOBAL_XS = Ref(:julia)
global_getfield() = callgetfield1(GLOBAL_XS)
@test let
Base.Experimental.@force_compile
global_getfield()
end === :julia
GLOBAL_XS[] = :julia2
@test let
Base.Experimental.@force_compile
global_getfield()
end === :julia2

# the `:inaccessiblememonly` helper effect allows us to prove `:effect_free`-ness of frames
# including `setfield!` modifying local mutable object

Expand Down