effects: refactor builtin_effects (JuliaLang#46097)

Factor special builtin handlings into separated functions
(e.g. `getfield_effects`) so that we can refactor them more easily.

This also improves analysis accuracy a bit, e.g.
```julia
julia> Base.infer_effects((Bool,)) do c
           obj = c ? Some{String}("foo") : Some{Symbol}(:bar)
           return getfield(obj, :value)
       end
(!c,+e,!n,+t,!s) # master
(+c,+e,!n,+t,+s) # this PR
```

Author: aviatesk

base/compiler/abstractinterpretation.jl

@@ -2161,16 +2161,15 @@ function abstract_eval_global(M::Module, s::Symbol)
 end
 
 function abstract_eval_global(M::Module, s::Symbol, frame::InferenceState)
-    ty = abstract_eval_global(M, s)
-    isa(ty, Const) && return ty
-    if isdefined(M,s)
-        tristate_merge!(frame, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE))
-    else
-        tristate_merge!(frame, Effects(EFFECTS_TOTAL;
-            consistent=ALWAYS_FALSE,
-            nothrow=ALWAYS_FALSE))
-    end
-    return ty
+    rt = abstract_eval_global(M, s)
+    consistent = nothrow = ALWAYS_FALSE
+    if isa(rt, Const)
+        consistent = nothrow = ALWAYS_TRUE
+    elseif isdefined(M,s)
+        nothrow = ALWAYS_TRUE
+    end
+    tristate_merge!(frame, Effects(EFFECTS_TOTAL; consistent, nothrow))
+    return rt
 end
 
 function handle_global_assignment!(interp::AbstractInterpreter, frame::InferenceState, lhs::GlobalRef, @nospecialize(newty))

base/compiler/tfuncs.jl

@@ -749,6 +749,7 @@ function getfield_boundscheck(argtypes::Vector{Any}) # ::Union{Bool, Nothing, Ty
     else
         return nothing
     end
+    isvarargtype(boundscheck) && return nothing
     widenconst(boundscheck) !== Bool && return nothing
     boundscheck = widenconditional(boundscheck)
     if isa(boundscheck, Const)
@@ -1850,67 +1851,82 @@ const _SPECIAL_BUILTINS = Any[
     Core._apply_iterate
 ]
 
+function isdefined_effects(argtypes::Vector{Any})
+    # consistent if the first arg is immutable
+    isempty(argtypes) && return EFFECTS_THROWS
+    obj = argtypes[1]
+    isvarargtype(obj) && return Effects(EFFECTS_THROWS; consistent=ALWAYS_FALSE)
+    consistent = is_immutable_argtype(obj) ? ALWAYS_TRUE : ALWAYS_FALSE
+    nothrow = isdefined_nothrow(argtypes) ? ALWAYS_TRUE : ALWAYS_FALSE
+    return Effects(EFFECTS_TOTAL; consistent, nothrow)
+end
+
+function getfield_effects(argtypes::Vector{Any}, @nospecialize(rt))
+    # consistent if the argtype is immutable
+    isempty(argtypes) && return EFFECTS_THROWS
+    obj = argtypes[1]
+    isvarargtype(obj) && return Effects(EFFECTS_THROWS; consistent=ALWAYS_FALSE)
+    consistent = is_immutable_argtype(obj) ? ALWAYS_TRUE : ALWAYS_FALSE
+    # access to `isbitstype`-field initialized with undefined value leads to undefined behavior
+    # so should taint `:consistent`-cy while access to uninitialized non-`isbitstype` field
+    # throws `UndefRefError` so doesn't need to taint it
+    # NOTE `getfield_notundefined` conservatively checks if this field is never initialized
+    # with undefined value so that we don't taint `:consistent`-cy too aggressively here
+    if !(length(argtypes) ≥ 2 && getfield_notundefined(widenconst(obj), argtypes[2]))
+        consistent = ALWAYS_FALSE
+    end
+    if getfield_boundscheck(argtypes) !== true
+        # If we cannot independently prove inboundsness, taint consistency.
+        # The inbounds-ness assertion requires dynamic reachability, while
+        # :consistent needs to be true for all input values.
+        # N.B. We do not taint for `--check-bounds=no` here -that happens in
+        # InferenceState.
+        if length(argtypes) ≥ 2 && getfield_nothrow(argtypes[1], argtypes[2], true)
+            nothrow = ALWAYS_TRUE
+        else
+            consistent = nothrow = ALWAYS_FALSE
+        end
+    else
+        nothrow = getfield_nothrow(argtypes) ? ALWAYS_TRUE : ALWAYS_FALSE
+    end
+    return Effects(EFFECTS_TOTAL; consistent, nothrow)
+end
+
+function getglobal_effects(argtypes::Vector{Any}, @nospecialize(rt))
+    consistent = nothrow = ALWAYS_FALSE
+    if getglobal_nothrow(argtypes)
+        # typeasserts below are already checked in `getglobal_nothrow`
+        M, s = (argtypes[1]::Const).val::Module, (argtypes[2]::Const).val::Symbol
+        if isconst(M, s)
+            consistent = nothrow = ALWAYS_TRUE
+        else
+            nothrow = ALWAYS_TRUE
+        end
+    end
+    return Effects(EFFECTS_TOTAL; consistent, nothrow)
+end
+
 function builtin_effects(f::Builtin, argtypes::Vector{Any}, @nospecialize(rt))
     if isa(f, IntrinsicFunction)
         return intrinsic_effects(f, argtypes)
     end
 
     @assert !contains_is(_SPECIAL_BUILTINS, f)
 
-    if (f === Core.getfield || f === Core.isdefined) && length(argtypes) >= 2
-        # consistent if the argtype is immutable
-        if isvarargtype(argtypes[1])
-            return Effects(; effect_free=ALWAYS_TRUE, terminates=ALWAYS_TRUE, nonoverlayed=true)
-        end
-        s = widenconst(argtypes[1])
-        if isType(s) || !isa(s, DataType) || isabstracttype(s)
-            return Effects(; effect_free=ALWAYS_TRUE, terminates=ALWAYS_TRUE, nonoverlayed=true)
-        end
-        s = s::DataType
-        consistent = !ismutabletype(s) ? ALWAYS_TRUE : ALWAYS_FALSE
-        # access to `isbitstype`-field initialized with undefined value leads to undefined behavior
-        # so should taint `:consistent`-cy while access to uninitialized non-`isbitstype` field
-        # throws `UndefRefError` so doesn't need to taint it
-        # NOTE `getfield_notundefined` conservatively checks if this field is never initialized
-        # with undefined value so that we don't taint `:consistent`-cy too aggressively here
-        if f === Core.getfield && !getfield_notundefined(s, argtypes[2])
-            consistent = ALWAYS_FALSE
-        end
-        if f === Core.getfield && !isvarargtype(argtypes[end]) && getfield_boundscheck(argtypes) !== true
-            # If we cannot independently prove inboundsness, taint consistency.
-            # The inbounds-ness assertion requires dynamic reachability, while
-            # :consistent needs to be true for all input values.
-            # N.B. We do not taint for `--check-bounds=no` here -that happens in
-            # InferenceState.
-            if getfield_nothrow(argtypes[1], argtypes[2], true)
-                nothrow = ALWAYS_TRUE
-            else
-                consistent = nothrow = ALWAYS_FALSE
-            end
-        else
-            nothrow = (!isvarargtype(argtypes[end]) && builtin_nothrow(f, argtypes, rt)) ?
-                ALWAYS_TRUE : ALWAYS_FALSE
-        end
-        effect_free = ALWAYS_TRUE
+    if f === isdefined
+        return isdefined_effects(argtypes)
+    elseif f === getfield
+        return getfield_effects(argtypes, rt)
     elseif f === getglobal
-        if getglobal_nothrow(argtypes)
-            consistent = isconst( # types are already checked in `getglobal_nothrow`
-                (argtypes[1]::Const).val::Module, (argtypes[2]::Const).val::Symbol) ?
-                ALWAYS_TRUE : ALWAYS_FALSE
-            nothrow = ALWAYS_TRUE
-        else
-            consistent = nothrow = ALWAYS_FALSE
-        end
-        effect_free = ALWAYS_TRUE
+        return getglobal_effects(argtypes, rt)
     else
         consistent = contains_is(_CONSISTENT_BUILTINS, f) ? ALWAYS_TRUE : ALWAYS_FALSE
         effect_free = (contains_is(_EFFECT_FREE_BUILTINS, f) || contains_is(_PURE_BUILTINS, f)) ?
             ALWAYS_TRUE : ALWAYS_FALSE
         nothrow = (!(!isempty(argtypes) && isvarargtype(argtypes[end])) && builtin_nothrow(f, argtypes, rt)) ?
             ALWAYS_TRUE : ALWAYS_FALSE
+        return Effects(EFFECTS_TOTAL; consistent, effect_free, nothrow)
     end
-
-    return Effects(EFFECTS_TOTAL; consistent, effect_free, nothrow)
 end
 
 function builtin_nothrow(@nospecialize(f), argtypes::Vector{Any}, @nospecialize(rt))

base/compiler/typeinfer.jl

@@ -429,7 +429,8 @@ function adjust_effects(sv::InferenceState)
     if !ipo_effects.inbounds_taints_consistency && rt === Bottom
         # always throwing an error counts or never returning both count as consistent
         ipo_effects = Effects(ipo_effects; consistent=ALWAYS_TRUE)
-    elseif ipo_effects.consistent === TRISTATE_UNKNOWN && is_consistent_rt(rt)
+    end
+    if ipo_effects.consistent === TRISTATE_UNKNOWN && is_consistent_argtype(rt)
         # in a case when the :consistent-cy here is only tainted by mutable allocations
         # (indicated by `TRISTATE_UNKNOWN`), we may be able to refine it if the return
         # type guarantees that the allocations are never returned
@@ -460,14 +461,6 @@ function adjust_effects(sv::InferenceState)
     return ipo_effects
 end
 
-is_consistent_rt(@nospecialize rt) = _is_consistent_rt(widenconst(ignorelimited(rt)))
-function _is_consistent_rt(@nospecialize ty)
-    if isa(ty, Union)
-        return _is_consistent_rt(ty.a) && _is_consistent_rt(ty.b)
-    end
-    return ty === Symbol || isbitstype(ty)
-end
-
 # inference completed on `me`
 # update the MethodInstance
 function finish(me::InferenceState, interp::AbstractInterpreter)

base/compiler/typeutils.jl

@@ -300,3 +300,24 @@ function unwraptv(@nospecialize t)
     end
     return t
 end
+
+# this query is specially written for `adjust_effects` and returns true if a value of this type
+# never involves inconsistency of mutable objects that are allocated somewhere within a call graph
+is_consistent_argtype(@nospecialize ty) = is_consistent_type(widenconst(ignorelimited(ty)))
+is_consistent_type(@nospecialize ty) = _is_consistent_type(unwrap_unionall(ty))
+function _is_consistent_type(@nospecialize ty)
+    if isa(ty, Union)
+        return is_consistent_type(ty.a) && is_consistent_type(ty.b)
+    end
+    # N.B. String and Symbol are mutable, but also egal always, and so they never be inconsistent
+    return ty === String || ty === Symbol || isbitstype(ty)
+end
+
+is_immutable_argtype(@nospecialize ty) = is_immutable_type(widenconst(ignorelimited(ty)))
+is_immutable_type(@nospecialize ty) = _is_immutable_type(unwrap_unionall(ty))
+function _is_immutable_type(@nospecialize ty)
+    if isa(ty, Union)
+        return is_immutable_type(ty.a) && is_immutable_type(ty.b)
+    end
+    return !isabstracttype(ty) && !ismutabletype(ty)
+end

test/compiler/effects.jl

@@ -295,7 +295,7 @@ end |> !Core.Compiler.is_nothrow
 # even if 2-arg `getfield` may throw, it should be still `:consistent`
 @test Core.Compiler.is_consistent(Base.infer_effects(getfield, (NTuple{5, Float64}, Int)))
 
-# SimpleVector allocation can be consistent
+# SimpleVector allocation is consistent
 @test Core.Compiler.is_consistent(Base.infer_effects(Core.svec))
 @test Base.infer_effects() do
     Core.svec(nothing, 1, "foo")
@@ -305,3 +305,12 @@ end |> Core.Compiler.is_consistent
 @test Base.infer_effects((Vector{Int},)) do a
     Base.@assume_effects :effect_free @ccall jl_array_ptr(a::Any)::Ptr{Int}
 end |> Core.Compiler.is_effect_free
+
+# `getfield_effects` handles union object nicely
+@test Core.Compiler.is_consistent(Core.Compiler.getfield_effects(Any[Some{String}, Core.Const(:value)], String))
+@test Core.Compiler.is_consistent(Core.Compiler.getfield_effects(Any[Some{Symbol}, Core.Const(:value)], Symbol))
+@test Core.Compiler.is_consistent(Core.Compiler.getfield_effects(Any[Union{Some{Symbol},Some{String}}, Core.Const(:value)], Union{Symbol,String}))
+@test Base.infer_effects((Bool,)) do c
+    obj = c ? Some{String}("foo") : Some{Symbol}(:bar)
+    return getfield(obj, :value)
+end |> Core.Compiler.is_consistent