implement setproperty! for modules (#44231)

This replaces #44137. As discussed on triage, instead of supporting
modules in `setfield!`, this adds two new builtins `getglobal` and
`setglobal!` explicitly for reading and modifying module bindings. We
should probably consider `getfield(::Module, ::Symbol)` to be
soft-deprecated, but I don't think we want to add any warnings since
that will likely just annoy people.

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Co-authored-by: Dilum Aluthge <dilum@aluthge.com>

Author: 3 people

NEWS.md

@@ -4,10 +4,14 @@ Julia v1.9 Release Notes
 New language features
 ---------------------
 
+* It is now possible to assign to bindings in another module using `setproperty!(::Module, ::Symbol, x)`. ([#44137])
 
 Language changes
 ----------------
 
+* New builtins `getglobal(::Module, ::Symbol[, order])` and `setglobal!(::Module, ::Symbol, x[, order])`
+  for reading from and writing to globals. `getglobal` should now be preferred for accessing globals over
+  `getfield`. ([#44137])
 
 Compiler/Runtime improvements
 -----------------------------

base/Base.jl

@@ -28,8 +28,7 @@ macro noinline() Expr(:meta, :noinline) end
 # Try to help prevent users from shooting them-selves in the foot
 # with ambiguities by defining a few common and critical operations
 # (and these don't need the extra convert code)
-getproperty(x::Module, f::Symbol) = (@inline; getfield(x, f))
-setproperty!(x::Module, f::Symbol, v) = setfield!(x, f, v) # to get a decent error
+getproperty(x::Module, f::Symbol) = (@inline; getglobal(x, f))
 getproperty(x::Type, f::Symbol) = (@inline; getfield(x, f))
 setproperty!(x::Type, f::Symbol, v) = error("setfield! fields of Types should not be changed")
 getproperty(x::Tuple, f::Int) = (@inline; getfield(x, f))
@@ -40,8 +39,12 @@ setproperty!(x, f::Symbol, v) = setfield!(x, f, convert(fieldtype(typeof(x), f),
 
 dotgetproperty(x, f) = getproperty(x, f)
 
-getproperty(x::Module, f::Symbol, order::Symbol) = (@inline; getfield(x, f, order))
-setproperty!(x::Module, f::Symbol, v, order::Symbol) = setfield!(x, f, v, order) # to get a decent error
+getproperty(x::Module, f::Symbol, order::Symbol) = (@inline; getglobal(x, f, order))
+function setproperty!(x::Module, f::Symbol, v, order::Symbol=:monotonic)
+    @inline
+    val::Core.get_binding_type(x, f) = v
+    return setglobal!(x, f, val, order)
+end
 getproperty(x::Type, f::Symbol, order::Symbol) = (@inline; getfield(x, f, order))
 setproperty!(x::Type, f::Symbol, v, order::Symbol) = error("setfield! fields of Types should not be changed")
 getproperty(x::Tuple, f::Int, order::Symbol) = (@inline; getfield(x, f, order))

base/boot.jl

@@ -193,6 +193,8 @@ export
     # object model functions
     fieldtype, getfield, setfield!, swapfield!, modifyfield!, replacefield!,
     nfields, throw, tuple, ===, isdefined, eval,
+    # access to globals
+    getglobal, setglobal!,
     # ifelse, sizeof    # not exported, to avoid conflicting with Base
     # type reflection
     <:, typeof, isa, typeassert,
@@ -201,7 +203,7 @@ export
     # constants
     nothing, Main
 
-const getproperty = getfield
+const getproperty = getfield # TODO: use `getglobal` for modules instead
 const setproperty! = setfield!
 
 abstract type Number end

base/compiler/abstractinterpretation.jl

@@ -1998,10 +1998,8 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e),
 end
 
 function abstract_eval_global(M::Module, s::Symbol)
-    if isdefined(M,s)
-        if isconst(M,s)
-            return Const(getfield(M,s))
-        end
+    if isdefined(M, s) && isconst(M, s)
+        return Const(getglobal(M, s))
     end
     ty = ccall(:jl_binding_type, Any, (Any, Any), M, s)
     ty === nothing && return Any

base/compiler/optimize.jl

@@ -675,7 +675,7 @@ function statement_cost(ex::Expr, line::Int, src::Union{CodeInfo, IRCode}, sptyp
             # The efficiency of operations like a[i] and s.b
             # depend strongly on whether the result can be
             # inferred, so check the type of ex
-            if f === Core.getfield || f === Core.tuple
+            if f === Core.getfield || f === Core.tuple || f === Core.getglobal
                 # we might like to penalize non-inferrability, but
                 # tuple iteration/destructuring makes that impossible
                 # return plus_saturate(argcost, isknowntype(extyp) ? 1 : params.inline_nonleaf_penalty)

base/compiler/ssair/passes.jl

@@ -418,7 +418,7 @@ function lift_leaves(compact::IncrementalCompact,
         elseif isa(leaf, GlobalRef)
             mod, name = leaf.mod, leaf.name
             if isdefined(mod, name) && isconst(mod, name)
-                leaf = getfield(mod, name)
+                leaf = getglobal(mod, name)
             else
                 return nothing
             end

base/compiler/tfuncs.jl

@@ -480,6 +480,37 @@ function arraysize_nothrow(argtypes::Vector{Any})
     return false
 end
 
+struct MemoryOrder x::Cint end
+const MEMORY_ORDER_UNSPECIFIED = MemoryOrder(-2)
+const MEMORY_ORDER_INVALID     = MemoryOrder(-1)
+const MEMORY_ORDER_NOTATOMIC   = MemoryOrder(0)
+const MEMORY_ORDER_UNORDERED   = MemoryOrder(1)
+const MEMORY_ORDER_MONOTONIC   = MemoryOrder(2)
+const MEMORY_ORDER_CONSUME     = MemoryOrder(3)
+const MEMORY_ORDER_ACQUIRE     = MemoryOrder(4)
+const MEMORY_ORDER_RELEASE     = MemoryOrder(5)
+const MEMORY_ORDER_ACQ_REL     = MemoryOrder(6)
+const MEMORY_ORDER_SEQ_CST     = MemoryOrder(7)
+
+function get_atomic_order(order::Symbol, loading::Bool, storing::Bool)
+    if order === :not_atomic
+        return MEMORY_ORDER_NOTATOMIC
+    elseif order === :unordered && (loading ⊻ storing)
+        return MEMORY_ORDER_UNORDERED
+    elseif order === :monotonic && (loading | storing)
+        return MEMORY_ORDER_MONOTONIC
+    elseif order === :acquire && loading
+        return MEMORY_ORDER_ACQUIRE
+    elseif order === :release && storing
+        return MEMORY_ORDER_RELEASE
+    elseif order === :acquire_release && (loading & storing)
+        return MEMORY_ORDER_ACQ_REL
+    elseif order === :sequentially_consistent
+        return MEMORY_ORDER_SEQ_CST
+    end
+    return MEMORY_ORDER_INVALID
+end
+
 function pointer_eltype(@nospecialize(ptr))
     a = widenconst(ptr)
     if !has_free_typevars(a)
@@ -1704,6 +1735,8 @@ function _builtin_nothrow(@nospecialize(f), argtypes::Array{Any,1}, @nospecializ
             return true
         end
         return false
+    elseif f === getglobal
+        return getglobal_nothrow(argtypes)
     elseif f === Core.get_binding_type
         length(argtypes) == 2 || return false
         return argtypes[1] ⊑ Module && argtypes[2] ⊑ Symbol
@@ -1721,7 +1754,7 @@ const _EFFECT_FREE_BUILTINS = [
     fieldtype, apply_type, isa, UnionAll,
     getfield, arrayref, const_arrayref, isdefined, Core.sizeof,
     Core.kwfunc, Core.ifelse, Core._typevar, (<:),
-    typeassert, throw, arraysize
+    typeassert, throw, arraysize, getglobal,
 ]
 
 const _CONSISTENT_BUILTINS = Any[
@@ -1774,16 +1807,20 @@ function builtin_effects(f::Builtin, argtypes::Vector{Any}, rt)
             # InferenceState.
             nothrow = getfield_nothrow(argtypes[2], argtypes[3], true)
             ipo_consistent &= nothrow
-        end
+        else
+            nothrow = isvarargtype(argtypes[end]) ? false :
+                builtin_nothrow(f, argtypes[2:end], rt)
+        end
+        effect_free = f === isdefined
+    elseif f === getglobal && length(argtypes) >= 3
+        nothrow = effect_free = getglobal_nothrow(argtypes[2:end])
+        ipo_consistent = nothrow && isconst((argtypes[2]::Const).val, (argtypes[3]::Const).val)
+        #effect_free = nothrow && isbindingresolved((argtypes[2]::Const).val, (argtypes[3]::Const).val)
     else
         ipo_consistent = contains_is(_CONSISTENT_BUILTINS, f)
+        effect_free = contains_is(_EFFECT_FREE_BUILTINS, f) || contains_is(_PURE_BUILTINS, f)
+        nothrow = isvarargtype(argtypes[end]) ? false : builtin_nothrow(f, argtypes[2:end], rt)
     end
-    # If we computed nothrow above for getfield, no need to repeat the procedure here
-    if !nothrow
-        nothrow = isvarargtype(argtypes[end]) ? false :
-            builtin_nothrow(f, argtypes[2:end], rt)
-    end
-    effect_free = contains_is(_EFFECT_FREE_BUILTINS, f) || contains_is(_PURE_BUILTINS, f)
 
     return Effects(
         ipo_consistent ? ALWAYS_TRUE : ALWAYS_FALSE,
@@ -2030,17 +2067,63 @@ function typename_static(@nospecialize(t))
     return isType(t) ? _typename(t.parameters[1]) : Core.TypeName
 end
 
+function global_order_nothrow(@nospecialize(o), loading::Bool, storing::Bool)
+    o isa Const || return false
+    sym = o.val
+    if sym isa Symbol
+        order = get_atomic_order(sym, loading, storing)
+        return order !== MEMORY_ORDER_INVALID && order !== MEMORY_ORDER_NOTATOMIC
+    end
+    return false
+end
+function getglobal_nothrow(argtypes::Vector{Any})
+    2 ≤ length(argtypes) ≤ 3 || return false
+    if length(argtypes) == 3
+        global_order_nothrow(o, true, false) || return false
+    end
+    M, s = argtypes
+    if M isa Const && s isa Const
+        M, s = M.val, s.val
+        if M isa Module && s isa Symbol
+            return isdefined(M, s)
+        end
+    end
+    return false
+end
+function getglobal_tfunc(@nospecialize(M), @nospecialize(s), @nospecialize(_=Symbol))
+    if M isa Const && s isa Const
+        M, s = M.val, s.val
+        if M isa Module && s isa Symbol
+            return abstract_eval_global(M, s)
+        end
+        return Bottom
+    elseif !(hasintersect(widenconst(M), Module) && hasintersect(widenconst(s), Symbol))
+        return Bottom
+    end
+    return Any
+end
+function setglobal!_tfunc(@nospecialize(M), @nospecialize(s), @nospecialize(v),
+                          @nospecialize(_=Symbol))
+    if !(hasintersect(widenconst(M), Module) && hasintersect(widenconst(s), Symbol))
+        return Bottom
+    end
+    return v
+end
+add_tfunc(getglobal, 2, 3, getglobal_tfunc, 1)
+add_tfunc(setglobal!, 3, 4, setglobal!_tfunc, 3)
+
 function get_binding_type_effect_free(@nospecialize(M), @nospecialize(s))
-    if M isa Const && widenconst(M) === Module &&
-        s isa Const && widenconst(s) === Symbol
-        return ccall(:jl_binding_type, Any, (Any, Any), M.val, s.val) !== nothing
+    if M isa Const && s isa Const
+        M, s = M.val, s.val
+        if M isa Module && s isa Symbol
+            return ccall(:jl_binding_type, Any, (Any, Any), M, s) !== nothing
+        end
     end
     return false
 end
 function get_binding_type_tfunc(@nospecialize(M), @nospecialize(s))
     if get_binding_type_effect_free(M, s)
-        @assert M isa Const && s isa Const
-        return Const(Core.get_binding_type(M.val, s.val))
+        return Const(Core.get_binding_type((M::Const).val, (s::Const).val))
     end
     return Type
 end

base/compiler/utilities.jl

@@ -52,7 +52,7 @@ function istopfunction(@nospecialize(f), name::Symbol)
     tn = typeof(f).name
     if tn.mt.name === name
         top = _topmod(tn.module)
-        return isdefined(top, name) && isconst(top, name) && f === getfield(top, name)
+        return isdefined(top, name) && isconst(top, name) && f === getglobal(top, name)
     end
     return false
 end

base/docs/basedocs.jl

@@ -2731,6 +2731,9 @@ The syntax `a.b = c` calls `setproperty!(a, :b, c)`.
 The syntax `@atomic order a.b = c` calls `setproperty!(a, :b, c, :order)`
 and the syntax `@atomic a.b = c` calls `getproperty(a, :b, :sequentially_consistent)`.
 
+!!! compat "Julia 1.8"
+    `setproperty!` on modules requires at least Julia 1.8.
+
 See also [`setfield!`](@ref Core.setfield!),
 [`propertynames`](@ref Base.propertynames) and
 [`getproperty`](@ref Base.getproperty).

doc/src/manual/variables-and-scoping.md

@@ -91,12 +91,6 @@ julia> module D
            b = a # errors as D's global scope is separate from A's
        end;
 ERROR: UndefVarError: a not defined
-
-julia> module E
-           import ..A # make module A available
-           A.a = 2    # throws below error
-       end;
-ERROR: cannot assign variables in other modules
 ```
 
 If a top-level expression contains a variable declaration with keyword `local`,

doc/src/manual/variables.md

@@ -81,13 +81,13 @@ julia> pi
 π = 3.1415926535897...
 
 julia> pi = 3
-ERROR: cannot assign a value to variable MathConstants.pi from module Main
+ERROR: cannot assign a value to imported variable MathConstants.pi from module Main
 
 julia> sqrt(100)
 10.0
 
 julia> sqrt = 4
-ERROR: cannot assign a value to variable Base.sqrt from module Main
+ERROR: cannot assign a value to imported variable Base.sqrt from module Main
 ```
 
 ## [Allowed Variable Names](@id man-allowed-variable-names)

src/builtin_proto.h

@@ -55,6 +55,7 @@ DECLARE_BUILTIN(_typebody);
 DECLARE_BUILTIN(typeof);
 DECLARE_BUILTIN(_typevar);
 DECLARE_BUILTIN(donotdelete);
+DECLARE_BUILTIN(getglobal);
 
 JL_CALLABLE(jl_f_invoke_kwsorter);
 #ifdef DEFINE_BUILTIN_GLOBALS
@@ -70,6 +71,7 @@ JL_CALLABLE(jl_f__equiv_typedef);
 JL_CALLABLE(jl_f_get_binding_type);
 JL_CALLABLE(jl_f_set_binding_type);
 JL_CALLABLE(jl_f_donotdelete);
+JL_CALLABLE(jl_f_setglobal);
 
 #ifdef __cplusplus
 }