Skip to content

Commit c750629

Browse files
committed
fix new type instability from getindex(::String, r::UnitRange{Int})
After investigating the cause of the invalidation reported in #55583, it was found that the issue arises only when `r` is propagated as an extended lattice element, such as `PartialStruct(UnitRange{Int}, ...)`, for the method of `getindex(::String, r::UnitRange{Int})`. Specifically, the path at https://github.com/JuliaLang/julia/blob/cebfd7bc66153b82c56715cb1cb52dac7df8eac8/base/compiler/typeinfer.jl#L809-L815 is hit, so the direct cause was the recursion limit for constant inference. To explain in more detail, within the slow path of `nextind` which is called inside `getindex(::String, ::UnitRange{Int})`, the 1-argument `@assert` is used https://github.com/JuliaLang/julia/blob/cebfd7bc66153b82c56715cb1cb52dac7df8eac8/base/strings/string.jl#L211. The code related to `print` associated with this `@assert` further uses `getindex(::String, ::UnitRange{Int})`, causing the recursion limit. This recursion limit is only for constant inference, which is why we saw this regression only for the `PartialStruct` case. Moreover, since this recursion limit occurs within the `@assert`-related code, this issue did not arise until now (i.e. until #49260 was merged). As a solution, I considered improving the recursion limit itself, but decided that keeping the current code for the recursion limit of constant inference is safer. Ideally, this should be addressed on the compiler side, but there is certainly deep recursion in this case. As an easier solution, this commit resolves the issue by changing the 1-arg `@assert` to the 2-arg version. - replaces #55583
1 parent cebfd7b commit c750629

2 files changed

Lines changed: 4 additions & 3 deletions

File tree

base/compiler/typeinfer.jl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
864864
end
865865
end
866866
if ccall(:jl_get_module_infer, Cint, (Any,), method.module) == 0 && !generating_output(#=incremental=#false)
867-
add_remark!(interp, caller, "Inference is disabled for the target module")
867+
add_remark!(interp, caller, "[typeinf_edge] Inference is disabled for the target module")
868868
return EdgeCallResult(Any, Any, nothing, Effects())
869869
end
870870
if !is_cached(caller) && frame_parent(caller) === nothing
@@ -897,7 +897,7 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
897897
end
898898
frame = InferenceState(result, cache_mode, interp) # always use the cache for edge targets
899899
if frame === nothing
900-
add_remark!(interp, caller, "Failed to retrieve source")
900+
add_remark!(interp, caller, "[typeinf_edge] Failed to retrieve source")
901901
# can't get the source for this, so we know nothing
902902
if cache_mode == CACHE_MODE_GLOBAL
903903
engine_reject(interp, ci)
@@ -918,6 +918,7 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
918918
return EdgeCallResult(frame.bestguess, exc_bestguess, edge, effects, volatile_inf_result)
919919
elseif frame === true
920920
# unresolvable cycle
921+
add_remark!(interp, caller, "[typeinf_edge] Unresolvable cycle")
921922
return EdgeCallResult(Any, Any, nothing, Effects())
922923
end
923924
# return the current knowledge about this cycle

base/strings/string.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ end
208208
i = i′
209209
@inbounds l = codeunit(s, i)
210210
(l < 0x80) | (0xf8 l) && return i+1
211-
@assert l >= 0xc0
211+
@assert l >= 0xc0 "invalid codeunit"
212212
end
213213
# first continuation byte
214214
(i += 1) > n && return i

0 commit comments

Comments
 (0)