Skip to content

Commit 58c1d51

Browse files
authored
[REPLCompletions] fix #52099 by adjusting effects of HAMT methods (#52331)
After looking into #52099, I discovered that it has the same root cause as #51548. Essentially, when a method that's not `!effect_free` is applied to a `Const` value concretized by the `REPLInterpreter`'s aggressive inference, since the `!effect_free` method will not be concretized, it will eventually lead to the `Const` representing unexpected object being returned. This commit tries to fix the specific problem reported in #52099 by enhancing the effects of `Base.HAMT` methods, so they're concrete-evaled. Admittedly, this is more of a quick fix than a complete solution, and not the best one, but it was the simplest. A better solution might involve implementing EA's handling of `Memory`-objects that allows the compiler to automatically prove `:effect_free` in more scenarios. But this would need a bigger overhaul, so I plan to tackle it in another PR.
1 parent bac3ba5 commit 58c1d51

File tree

4 files changed

+18
-7
lines changed

4 files changed

+18
-7
lines changed

base/compiler/optimize.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ function scan_non_dataflow_flags!(inst::Instruction, sv::PostOptAnalysisState)
698698
flag = inst[:flag]
699699
# If we can prove that the argmem does not escape the current function, we can
700700
# refine this to :effect_free.
701-
needs_ea_validation = (flag & IR_FLAGS_NEEDS_EA) == IR_FLAGS_NEEDS_EA
701+
needs_ea_validation = has_flag(flag, IR_FLAGS_NEEDS_EA)
702702
stmt = inst[:stmt]
703703
if !needs_ea_validation
704704
if !isterminator(stmt) && stmt !== nothing
@@ -730,7 +730,7 @@ end
730730

731731
function scan_inconsistency!(inst::Instruction, sv::PostOptAnalysisState)
732732
flag = inst[:flag]
733-
stmt_inconsistent = iszero(flag & IR_FLAG_CONSISTENT)
733+
stmt_inconsistent = !has_flag(flag, IR_FLAG_CONSISTENT)
734734
stmt = inst[:stmt]
735735
# Special case: For `getfield` and memory operations, we allow inconsistency of the :boundscheck argument
736736
(; inconsistent, tpdum) = sv

base/hamt.jl

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,21 +66,21 @@ mutable struct HAMT{K, V}
6666
HAMT{K, V}() where {K, V} = new{K,V}(Vector{Union{Leaf{K, V}, HAMT{K, V}}}(undef, 0), zero(BITMAP))
6767
end
6868

69-
@Base.assume_effects :nothrow function init_hamt(K, V, k, v)
70-
# For a single element we can't have a hash-collision
69+
Base.@assume_effects :nothrow :effect_free function init_hamt(K, V, k, v)
70+
# For a single element we can't have a 'hash-collision
7171
trie = HAMT{K,V}(Vector{Union{Leaf{K, V}, HAMT{K, V}}}(undef, 1), zero(BITMAP))
7272
trie.data[1] = Leaf{K,V}(k,v)
7373
return trie
7474
end
7575

76-
function HAMT{K,V}((k,v)::Pair) where {K, V}
77-
k = convert(K, k)
78-
v = convert(V, v)
76+
Base.@assume_effects :effect_free function HAMT{K,V}((k,v)::Pair{K,V}) where {K, V}
7977
trie = init_hamt(K, V, k, v)
8078
bi = BitmapIndex(HashState(k))
8179
set!(trie, bi)
8280
return trie
8381
end
82+
HAMT{K,V}(kv::Pair) where {K, V} = HAMT{K,V}(convert(Pair{K,V}, kv))
83+
8484
HAMT(pair::Pair{K,V}) where {K, V} = HAMT{K,V}(pair)
8585

8686
# TODO: Parameterize by hash function

stdlib/REPL/test/replcompletions.jl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2148,3 +2148,11 @@ end
21482148
let t = REPLCompletions.repl_eval_ex(:(unsafe_method(42)), @__MODULE__)
21492149
@test isnothing(t)
21502150
end
2151+
2152+
# https://github.com/JuliaLang/julia/issues/52099
2153+
const issue52099 = []
2154+
let t = REPLCompletions.repl_eval_ex(:(Base.PersistentDict(issue52099 => 3)), @__MODULE__)
2155+
if t isa Core.Const
2156+
@test length(t.val) == 1
2157+
end
2158+
end

test/dict.jl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,9 @@ import Base.PersistentDict
11071107
@test hs.hash == hsr.hash
11081108
@test hs.depth == hsr.depth
11091109
@test hs.shift == hsr.shift
1110+
1111+
@test Core.Compiler.is_removable_if_unused(Base.infer_effects(Base.HAMT.init_hamt, (Type{Vector{Any}},Type{Int},Vector{Any},Int)))
1112+
@test Core.Compiler.is_removable_if_unused(Base.infer_effects(Base.HAMT.HAMT{Vector{Any},Int}, (Pair{Vector{Any},Int},)))
11101113
end
11111114
@testset "basics" begin
11121115
dict = PersistentDict{Int, Int}()

0 commit comments

Comments
 (0)