- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Lower const x = ...
to new builtin Core.setconst!
#58187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Some changes are needed to get Revise working again. JuliaInterpreter (just running diff --git i/src/builtins.jl w/src/builtins.jl
index 36ae00a..2168e11 100644
--- i/src/builtins.jl
+++ w/src/builtins.jl
@@ -224,6 +224,8 @@ function maybe_evaluate_builtin(frame, call_expr, expand::Bool)
else
return Some{Any}(Core.memoryrefswap!(getargs(args, frame)...))
end
+ elseif f === Core.setconst!
+ return Some{Any}(Core.setconst!(getargs(args, frame)...))
elseif f === Core.sizeof
if nargs == 1
return Some{Any}(Core.sizeof(lookup(frame, args[2])))
diff --git i/src/interpret.jl w/src/interpret.jl
index c552bd4..4744554 100644
--- i/src/interpret.jl
+++ w/src/interpret.jl
@@ -528,7 +528,7 @@ function step_expr!(@nospecialize(recurse), frame::Frame, @nospecialize(node), i
error("this should have been handled by split_expressions")
elseif node.head === :using || node.head === :import || node.head === :export
Core.eval(moduleof(frame), node)
- elseif node.head === :const || node.head === :globaldecl
+ elseif node.head === :globaldecl
g = node.args[1]
if length(node.args) == 2
Core.eval(moduleof(frame), Expr(:block, Expr(node.head, g, lookup(frame, node.args[2])), nothing)) LoweredCodeUtils: diff --git i/src/codeedges.jl w/src/codeedges.jl
index 647eae8..246e83a 100644
--- i/src/codeedges.jl
+++ w/src/codeedges.jl
@@ -196,7 +196,7 @@ end
# The depwarn is currently disabled because Revise's tests check for empty warning logs
function is_assignment_like(@nospecialize stmt)
# Base.depwarn("is_assignment_like is deprecated, switch to `LoweredCodeUtils.get_lhs_rhs`", :is_assignment_like)
- return isexpr(stmt, :(=)) || isexpr(stmt, :const, 2)
+ return isexpr(stmt, :(=))
end
function get_lhs_rhs(@nospecialize stmt)
@@ -206,7 +206,7 @@ function get_lhs_rhs(@nospecialize stmt)
return Pair{Any,Any}(stmt.args[1], stmt.args[2])
elseif isexpr(stmt, :call) && length(stmt.args) == 4
f = stmt.args[1]
- if is_global_ref_egal(f, :setglobal!, Core.setglobal!)
+ if is_global_ref_egal(f, :setglobal!, Core.setglobal!) || is_global_ref_egal(f, :setconst!, Core.setconst!)
mod = stmt.args[2]
mod isa Module || return nothing
name = stmt.args[3] |
src/julia-syntax.scm
Outdated
;; (= (globalref _ _) _) => setglobal! | ||
;; (const (globalref _ _) _) => setconst! | ||
(cond ((and (globalref? lhs) (eq? op '=)) | ||
(emit `(call (top setglobal!) ,(cadr lhs) (inert ,(caddr lhs)) ,rhs))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect this should also be (core setglobal!)
base/docs/basedocs.jl
Outdated
setglobalonce! | ||
|
||
""" | ||
setconst!(module::Module, name::Symbol, [x]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bikeshed: I think declare_const
might make more sense than calling it "set!"? Later we can add declare_method
similarly. They also do not modify anything (they create a new world for possible execution, but do not change anything), so they likely should not have a !
(similar to Base.delete_method
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, and I hadn't really considered how this would fit with possible builtins for the :global/:globaldecl
and :method
Exprs.
declare_global
matches jl_declare_global
, but we use noun-verb for jl_method_def
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, in other languages (llvm, C particularly) this would be define, not declare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also might want to choose the name to be consistent with _eval_import
, whether we rename that to declare_import
or rename this to eval_global
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to go with declare_const
and declare_global
for the time being. Ending in !
is just wrong, and we've got jl_declare_*
for internal stuff that establishes new bindings. I will defer the naming of functions that manipulate method tables to future bikeshedding.
Adds a new builtin, Core.setconst!, with the following signature: setconst!(module::Module, name::Symbol, [x]) The lowered Expr(:const, :name, value) form has been removed, saving some duplicated effort in the interpreter and code generation. Expr(:const, ...) is now always lowered to the builtin. The single-argument form of const, having no surface syntax, was previously only available through eval(Expr(:const, :foo)). It can now also be used by omitting the value argument to setconst!.
Could you update Revise so we can get this over the finish line? For a similar example, take a look at timholy/Revise.jl#890 |
Co-authored-by: Jameson Nash <[email protected]>
Core.declare_const doesn't mutate state and so shouldn't end with !. This also brings it in line with potential future builtins like Core.declare_global.
For what it's worth, I'm very much in favor of removing special What's left to get this over the line? |
This isn't ideal, but is required to have inference run in the same situations as before.
Adds a new builtin, `Core.setconst!`, with the following signature: ``` setconst!(module::Module, name::Symbol, [x]) ``` The lowered `Expr(:const, :name, value)` form has been removed, saving some duplicated effort in the interpreter and code generation. `Expr(:const, ...)` is now always lowered to the builtin. ``` julia> Meta.@lower const example = 123 :($(Expr(:thunk, CodeInfo( 1 ─ %1 = 123 │ builtin Core.setconst!(Main, :example, %1) │ $(Expr(:latestworld)) └── return %1 )))) ``` The single-argument form of const, having no surface syntax, was previously only available through `eval(Expr(:const, :foo))`: ``` julia> eval(Expr(:const, :example)) julia> GlobalRef(Main, :example).binding Binding Main.example 38582:∞ - undefined const binding 38579:38581 - undefined binding - guard entry ``` It survived lowering by being special-cased in `expand-toplevel-expr--`, resulting in some inconsistencies: ``` julia> eval(:(begin $(Expr(:const, :example)) end)) ERROR: syntax: malformed expression Stacktrace: [1] top-level scope @ none:1 [2] eval(m::Module, e::Any) @ Core ./boot.jl:489 [3] top-level scope @ REPL[1]:1 ``` These are fixed now that const is always lowered. --------- Co-authored-by: Jameson Nash <[email protected]>
…#58279) # Overview In the spirit of #58187 and #57965, this PR lowers more surface syntax to calls, eliminating the lowered `:global` and `:globaldecl` operations in favour of a single `Core.declare_global` builtin. `Core.declare_global` has the signature: ``` declare_global(module::Module, name::Symbol, strong::Bool=false, [ty::Type]) ``` - When `strong = false`, it has the effect of `global name` at the top level (see the description for [`PARTITION_KIND_DECLARED`](https://github.com/JuliaLang/julia/blob/d46b665067bd9fc352c89c9d0abb591eaa4f7695/src/julia.h#L706-L710)). - With `strong = true`: - No `ty` provided: if no global exists, creates a strong global with type `Any`. Has no effect if one already exists. This form is generated by global assignments with no type declaration. - `ty` provided: always creates a new global with the given type, failing if one already exists with a different declared type. ## Definition effects One of the purposes of this change is to remove the definitions effects for `:global` and `:globaldecl`: https://github.com/JuliaLang/julia/blob/d46b665067bd9fc352c89c9d0abb591eaa4f7695/src/method.c#L95-L105 The eventual goal is to make all the definition effects for a method explicit after lowering, simplifying interpreters for lowered IR. ## Minor lowering changes ### `global` permitted in more places Adds a new ephemeral syntax head, `unused-only`, to wrap expressions whose result should not be used. It generates the `misplaced "global" declaration` error, and is slightly more forgiving than the old restriction. This was necessary to permit `global` to be lowered in all contexts. Old: ``` julia> global example julia> begin global example end ERROR: syntax: misplaced "global" declaration Stacktrace: [1] top-level scope @ REPL[2]:1 ``` New: ``` julia> global example julia> begin global example end ``` ### `global` always lowered This change maintains support for some expressions that cannot be produced by the parser (similar to `Expr(:const, :foo)`): https://github.com/JuliaLang/julia/blob/d46b665067bd9fc352c89c9d0abb591eaa4f7695/test/precompile.jl#L2036 This used to work by bypassing lowering but is now lowered to the appropriate `declare_global` call. ## Generated functions After lowering the body AST returned by a `@generated` function, the definition effects are still performed. Instead of relying on a check in `jl_declare_global` to fail during this process, `GeneratedFunctionStub` now wraps the AST in a new Expr head, `Expr(:toplevel_pure, ...)`, indicating lowering should not produce toplevel side effects. Currently, this is used only to omit calls to `declare_global` for generated functions, but it could also be used to improve the catch-all error message when lowering returns a thunk (telling the user if it failed because of a closure, generator, etc), or even to support some closures by making them opaque. The error message for declaring a global as a side effect of a `@generated` function AST has changed, because it now fails when the assignment to an undeclared global is performed. Old: ``` julia> @generated function foo(x) :(global bar = x) end foo (generic function with 1 method) julia> foo(1) ERROR: new strong globals cannot be created in a generated function. Declare them outside using `global x::Any`. Stacktrace: [1] top-level scope @ REPL[2]:1 ``` New: ``` julia> @generated function foo(x) :(global bar = x) end foo (generic function with 1 method) julia> foo(1) ERROR: Global Main.bar does not exist and cannot be assigned. Note: Julia 1.9 and 1.10 inadvertently omitted this error check (#56933). Hint: Declare it using `global bar` inside `Main` before attempting assignment. Stacktrace: [1] macro expansion @ ./REPL[1]:1 [inlined] [2] foo(x::Int64) @ Main ./REPL[1]:1 [3] top-level scope @ REPL[2]:1 ``` ## Examples of the new lowering Toplevel weak global: ``` julia> Meta.@lower global example :($(Expr(:thunk, CodeInfo( 1 ─ builtin Core.declare_global(Main, :example, false) │ $(Expr(:latestworld)) └── return nothing )))) ``` Toplevel strong global declaration with type: ``` julia> Meta.@lower example::Int :($(Expr(:thunk, CodeInfo( 1 ─ %1 = Main.example │ %2 = Main.Int │ %3 = builtin Core.typeassert(%1, %2) └── return %3 )))) ``` Toplevel strong global assignment: ``` julia> Meta.@lower example = 1 :($(Expr(:thunk, CodeInfo( 1 ─ builtin Core.declare_global(Main, :example, true) │ $(Expr(:latestworld)) │ %3 = builtin Core.get_binding_type(Main, :example) │ #s1 = 1 │ %5 = #s1 │ %6 = builtin %5 isa %3 └── goto #3 if not %6 2 ─ goto #4 3 ─ %9 = #s1 └── #s1 = Base.convert(%3, %9) 4 ┄ %11 = #s1 │ dynamic Base.setglobal!(Main, :example, %11) └── return 1 )))) ``` Toplevel strong global assignment with type: ``` julia> Meta.@lower example::Int = 1 :($(Expr(:thunk, CodeInfo( 1 ─ %1 = Main.Int │ builtin Core.declare_global(Main, :example, true, %1) │ $(Expr(:latestworld)) │ %4 = builtin Core.get_binding_type(Main, :example) │ #s1 = 1 │ %6 = #s1 │ %7 = builtin %6 isa %4 └── goto #3 if not %7 2 ─ goto #4 3 ─ %10 = #s1 └── #s1 = Base.convert(%4, %10) 4 ┄ %12 = #s1 │ dynamic Base.setglobal!(Main, :example, %12) └── return 1 )))) ``` Global assignment inside function (call to `declare_global` hoisted to top level): ``` julia> Meta.@lower function f1(x) global example = x end :($(Expr(:thunk, CodeInfo( 1 ─ $(Expr(:method, :(Main.f1))) │ $(Expr(:latestworld)) │ $(Expr(:latestworld)) │ builtin Core.declare_global(Main, :example, false) │ $(Expr(:latestworld)) │ builtin Core.declare_global(Main, :example, true) │ $(Expr(:latestworld)) │ %8 = Main.f1 │ %9 = dynamic Core.Typeof(%8) │ %10 = builtin Core.svec(%9, Core.Any) │ %11 = builtin Core.svec() │ %12 = builtin Core.svec(%10, %11, $(QuoteNode(:(#= REPL[7]:1 =#)))) │ $(Expr(:method, :(Main.f1), :(%12), CodeInfo( @ REPL[7]:2 within `unknown scope` 1 ─ %1 = x │ %2 = builtin Core.get_binding_type(Main, :example) │ @_3 = %1 │ %4 = @_3 │ %5 = builtin %4 isa %2 └── goto #3 if not %5 2 ─ goto #4 3 ─ %8 = @_3 └── @_3 = Base.convert(%2, %8) 4 ┄ %10 = @_3 │ dynamic Base.setglobal!(Main, :example, %10) └── return %1 ))) │ $(Expr(:latestworld)) │ %15 = Main.f1 └── return %15 )))) ``` --------- Co-authored-by: Jameson Nash <[email protected]>
Adds a new builtin,
Core.setconst!
, with the following signature:The lowered
Expr(:const, :name, value)
form has been removed, saving someduplicated effort in the interpreter and code generation.
Expr(:const, ...)
isnow always lowered to the builtin.
The single-argument form of const, having no surface syntax, was previously only
available through
eval(Expr(:const, :foo))
:It survived lowering by being special-cased in
expand-toplevel-expr--
,resulting in some inconsistencies:
These are fixed now that const is always lowered.