Skip to content

Commit

Permalink
fix #52531, fix the effects modeling of QuoteNode (#52548)
Browse files Browse the repository at this point in the history
What observed in #52531 is that `QuoteNode` can embed global variables
that users can modify. Therefore, when dealing with `QuoteNode`, it's
necessary to taint its `:inaccessiblememonly` just like we do for
`GlobalRef`.

- fixes #52531
- replaces #52536
  • Loading branch information
aviatesk committed Dec 15, 2023
1 parent e8576fc commit e207c10
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
4 changes: 3 additions & 1 deletion base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2306,7 +2306,9 @@ end

function abstract_eval_special_value(interp::AbstractInterpreter, @nospecialize(e), vtypes::Union{VarTable,Nothing}, sv::AbsIntState)
if isa(e, QuoteNode)
return RTEffects(Const(e.value), Union{}, EFFECTS_TOTAL)
effects = Effects(EFFECTS_TOTAL;
inaccessiblememonly = is_mutation_free_argtype(typeof(e.value)) ? ALWAYS_TRUE : ALWAYS_FALSE)
return RTEffects(Const(e.value), Union{}, effects)
elseif isa(e, SSAValue)
return RTEffects(abstract_eval_ssavalue(e, sv), Union{}, EFFECTS_TOTAL)
elseif isa(e, SlotNumber)
Expand Down
16 changes: 16 additions & 0 deletions test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1340,3 +1340,19 @@ end
end
return res
end |> Core.Compiler.is_terminates

# https://github.com/JuliaLang/julia/issues/52531
const a52531 = Core.Ref(1)
@eval getref52531() = $(QuoteNode(a52531)).x
@test !Core.Compiler.is_consistent(Base.infer_effects(getref52531))
let
global set_a52531!, get_a52531
_a::Int = -1
set_a52531!(a::Int) = (_a = a; return get_a52531())
get_a52531() = _a
end
@test !Core.Compiler.is_consistent(Base.infer_effects(set_a52531!, (Int,)))
@test !Core.Compiler.is_consistent(Base.infer_effects(get_a52531, ()))
@test get_a52531() == -1
@test set_a52531!(1) == 1
@test get_a52531() == 1

0 comments on commit e207c10

Please sign in to comment.