lowering: Refactor lowering for const and typed globals #54773
Conversation
|
DNM, because the base PR should be merged first. |
Not entirely true, since loading a module is a function call, but has a world-age change side effect. Similarly, deleting a method is a function call, but has a world-age change side effect. So 2 out of 3 world-change side effects I can think of are function calls, and only adding a method is currently a syntactic-only form. |
I meant world-age affecting syntax forms, which is currently |
Keno
force-pushed
the
kf/constgloballowerrefactor
branch
from
a6b3ce0 to
17320fa
Compare
|
Minor nit: currently
When? Currently functions are not allowed to observe new worlds, while global scope does so, but it also does update the world within a block of code to make it permitted |
|
Fair enough, the world age update at global scope is finer than I had thought (I thought it captured it at the entry to thunk evaluation, but it actually updates it every statement). I'm happy to go the other way and make both of these lower to intrinsics instead. Would you prefer that? |
|
Eventually, I think that will be desirable, though it is unimportant for the PR since that is a new feature it can be added later without breaking the syntactic form |
|
Alright, I'll leave it as is then and tweak the commit message. |
Keno
force-pushed
the
kf/constgloballowerrefactor
branch
from
17320fa to
8816e4f
Compare
Keno
added
needs docs
Keno
force-pushed
the
kf/constgloballowerrefactor
branch
3 times, most recently
from
df97a48 to
dfaa660
Compare
|
@nanosoldier |
|
The package evaluation job you requested has completed - possible new issues were detected. |
Keno
force-pushed
the
kf/constgloballowerrefactor
branch
from
dfaa660 to
717283e
Compare
|
Fixed the |
|
@nanosoldier |
|
The package evaluation job you requested has completed - possible new issues were detected. |
Keno
force-pushed
the
kf/constgloballowerrefactor
branch
from
717283e to
499eed2
Compare
|
@nanosoldier |
This is a follow up to resolve a TODO left in #54773 as part of preparatory work for #54654. Currently, our lowering for type definition contains an early `isdefined` that forces a decision on binding resolution before the assignment of the actual binding. In the current implementation, this doesn't matter much, but with #54654, this would incur a binding invalidation we would like to avoid. To get around this, we extend the (internal) `isdefined` form to take an extra argument specifying whether or not to permit looking at imported bindings. If not, resolving the binding is not required semantically, but for the purposes of type definition (where assigning to an imported binding would error anyway), this is all we need.
This is a follow up to resolve a TODO left in #54773 as part of preparatory work for #54654. Currently, our lowering for type definition contains an early `isdefined` that forces a decision on binding resolution before the assignment of the actual binding. In the current implementation, this doesn't matter much, but with #54654, this would incur a binding invalidation we would like to avoid. To get around this, we extend the (internal) `isdefined` form to take an extra argument specifying whether or not to permit looking at imported bindings. If not, resolving the binding is not required semantically, but for the purposes of type definition (where assigning to an imported binding would error anyway), this is all we need.
This is a follow up to resolve a TODO left in #54773 as part of preparatory work for #54654. Currently, our lowering for type definition contains an early `isdefined` that forces a decision on binding resolution before the assignment of the actual binding. In the current implementation, this doesn't matter much, but with #54654, this would incur a binding invalidation we would like to avoid. To get around this, we extend the (internal) `isdefined` form to take an extra argument specifying whether or not to permit looking at imported bindings. If not, resolving the binding is not required semantically, but for the purposes of type definition (where assigning to an imported binding would error anyway), this is all we need.
|
Regarding the newly added |
|
Just a side note: the fact that the world-age of top-level thunks can be incremented with each statement might mean that new considerations are necessary for the correctness of inference on top-level chunks. |
Following up #54773. Required for external abstract interpreters that may run inference on arbitrary top-level thunks.
Following up #54773. Required for external abstract interpreters that may run inference on arbitrary top-level thunks.
…HS to be global, and prohibit "const x[] = 1" (#57470) The changes to `const` resulted in confusing error messages when it was used inside a function (#57334). On 1.11.3: ``` julia> function f() const x = 1 end ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 Stacktrace: [1] top-level scope @ REPL[1]:1 ``` On nightly: ``` julia> function f() const x = 1 end ERROR: syntax: World age increment not at top level Stacktrace: [1] top-level scope @ REPL[1]:1 ``` In prior versions, we also accepted confused expressions like: ``` x = Ref(1) const x[] = 1 ``` This change adds a new error messages explicitly prohibiting `const` where the left hand side is not declaring variables: ``` ERROR: syntax: `const` left hand side "x[]" contains non-variables around REPL[2]:1 Stacktrace: [1] top-level scope @ REPL[2]:1 ``` Finally, #54773 made `const` stop participating in scope resolution (the left hand side was always taken to be in global scope). Some expressions that were prohibited started being accepted: In 1.11.3: ``` julia> let const x = 1 end ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 Stacktrace: [1] top-level scope @ REPL[1]:1 ``` Nightly: ``` julia> let const x = 1 end 1 ``` This change rejects `const` unless the variable would be in global scope (`global const` would be required in the example), so we don't lose the ability to make `const` in local scope meaningful later.
…HS to be global, and prohibit "const x[] = 1" (#57470) The changes to `const` resulted in confusing error messages when it was used inside a function (#57334). On 1.11.3: ``` julia> function f() const x = 1 end ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 Stacktrace: [1] top-level scope @ REPL[1]:1 ``` On nightly: ``` julia> function f() const x = 1 end ERROR: syntax: World age increment not at top level Stacktrace: [1] top-level scope @ REPL[1]:1 ``` In prior versions, we also accepted confused expressions like: ``` x = Ref(1) const x[] = 1 ``` This change adds a new error messages explicitly prohibiting `const` where the left hand side is not declaring variables: ``` ERROR: syntax: `const` left hand side "x[]" contains non-variables around REPL[2]:1 Stacktrace: [1] top-level scope @ REPL[2]:1 ``` Finally, #54773 made `const` stop participating in scope resolution (the left hand side was always taken to be in global scope). Some expressions that were prohibited started being accepted: In 1.11.3: ``` julia> let const x = 1 end ERROR: syntax: unsupported `const` declaration on local variable around REPL[1]:2 Stacktrace: [1] top-level scope @ REPL[1]:1 ``` Nightly: ``` julia> let const x = 1 end 1 ``` This change rejects `const` unless the variable would be in global scope (`global const` would be required in the example), so we don't lose the ability to make `const` in local scope meaningful later. (cherry picked from commit fb01f91)
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57626. Some changes omitted from `expand-decls` and `expand-assignment`
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57626. Some changes omitted from `expand-decls` and `expand-assignment`
An unfortunately large commit. JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57626. Some changes omitted from `expand-decls` and `expand-assignment`.
An unfortunately large commit. JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57626. Some changes omitted from `expand-decls` and `expand-assignment`.
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57626. Some changes omitted from `expand-decls` and `expand-assignment`.
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`.
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`.
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`.
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`.
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`.
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`. Note that the two-argument IR "const" is K"constdecl", whereas the one-argument K"const" only appears in the AST. Also note that the `wrap` parameter is omitted throughout assignment desugaring. As far as I'm aware, all this plumbing was just to support `const a,b,c = 1,2,3` having `b` and `c` inherit the `const`. TODO: find a better way of doing the same thing (a ScopedValue might be a clean solution; we currently throw an error). The check for `let; const x = 1; end`, (which should throw) is in scope analysis (lisp has it in `compile`). Co-authored-by: Claire Foster <[email protected]>
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`. Note that the two-argument IR "const" is K"constdecl", whereas the one-argument K"const" only appears in the AST. Also note that the `wrap` parameter is omitted throughout assignment desugaring. As far as I'm aware, all this plumbing was just to support `const a,b,c = 1,2,3` having `b` and `c` inherit the `const`. TODO: find a better way of doing the same thing (a ScopedValue might be a clean solution; we currently throw an error). The check for `let; const x = 1; end`, (which should throw) is in scope analysis (lisp has it in `compile`). Co-authored-by: Claire Foster <[email protected]>
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`. Note that the two-argument IR "const" is K"constdecl", whereas the one-argument K"const" only appears in the AST. Also note that the `wrap` parameter is omitted throughout assignment desugaring. As far as I'm aware, all this plumbing was just to support `const a,b,c = 1,2,3` having `b` and `c` inherit the `const`. TODO: find a better way of doing the same thing (a ScopedValue might be a clean solution; we currently throw an error). The check for `let; const x = 1; end`, (which should throw) is in scope analysis (lisp has it in `compile`). Co-authored-by: Claire Foster <[email protected]>
JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`. Note that the two-argument IR "const" is K"constdecl", whereas the one-argument K"const" only appears in the AST. Also note that the `wrap` parameter is omitted throughout assignment desugaring. As far as I'm aware, all this plumbing was just to support `const a,b,c = 1,2,3` having `b` and `c` inherit the `const`. TODO: find a better way of doing the same thing (a ScopedValue might be a clean solution; we currently throw an error). The check for `let; const x = 1; end`, (which should throw) is in scope analysis (lisp has it in `compile`). Co-authored-by: Claire Foster <[email protected]>
* Update CodeInfo struct and handling Co-authored-by: Claire Foster <[email protected]> * Don't produce raw symbol from globalref This used to implicitly refer to a module-level name, but lowering is now expected to wrap it in a `globalref`. Part of JuliaLang/julia#54772 * Updates to const and global lowering; add K"constdecl"; omit `wrap` JuliaLang/julia#54773, JuliaLang/julia#56713, JuliaLang/julia#57470. Some changes omitted from `expand-decls` and `expand-assignment`. Note that the two-argument IR "const" is K"constdecl", whereas the one-argument K"const" only appears in the AST. Also note that the `wrap` parameter is omitted throughout assignment desugaring. As far as I'm aware, all this plumbing was just to support `const a,b,c = 1,2,3` having `b` and `c` inherit the `const`. TODO: find a better way of doing the same thing (a ScopedValue might be a clean solution; we currently throw an error). The check for `let; const x = 1; end`, (which should throw) is in scope analysis (lisp has it in `compile`). Co-authored-by: Claire Foster <[email protected]> * Add `isdefinedglobal` builtin JuliaLang/julia#54999, JuliaLang/julia#56985 * :global no longer valid_ir_argument; rm `is_defined_nothrow_global` JuliaLang/julia#56746. Also call :slot and :static_parameter valid (for now) * Fix `is_defined_and_owned_global` (Core.Binding changes) Adapt to bpart changes in JuliaLang/julia#54788 * Struct desugaring: "Undo decision to publish incomplete types..." JuliaLang/julia#56497; Add self-referencing struct shim I have doubts about how long this solution will stay in the base repository, and how complete it is (doesn't seem to work with M1.M2.S), but we are testing for it here. Also change the expected value of a test changed in the same PR. * Emit `latestworld` world age increments For method defs, `latestworld` is produced in desugaring rather than closure conversion for now (our closure conversion doesn't seem to cover the same cases as lisp lowering yet). Covers JuliaLang/julia#56523, JuliaLang/julia#56509, JuliaLang/julia#57299. Also includes changes from JuliaLang/julia#57102 (bpart: Start enforcing minimum world age for const bparts) and JuliaLang/julia#57150 (bpart: Start enforcing min_world for global variable definitions) since the lowering changes from those appear to be amendments to the changes above (missing world age increments). Co-authored-by: Claire Foster <[email protected]> * bpart changes: `Core._typebody!` signature `Core._typebody!` now takes a new "prev" argument, which we don't use yet here. Changes from JuliaLang/julia#57253 * bpart changes: struct desugaring Changes from JuliaLang/julia#57253 (bpart: Fully switch to partitioned semantics). This fixes one failing test and realigns struct desugaring to match lisp for now. Also changed: the expected result of redefining a primitive type (now allowed). * Additional argument in `new_opaque_closure` Fix segfaulting test. Thanks for the TODO * Adapt to different `GeneratedFunctionStub` signature Signature changed in JuliaLang/julia#57230. Thanks @aviatesk for the help! * Fix `public` and `export` As of JuliaLang/julia#57765, `jl_module_public` is no longer exported. Change our runtime to handle it like `public` and `export` like we handle `import` or `using` for now * Fix modules.jl test I believe this was a world age issue * Regenerate IR tests Too many to count. * Update README to known-good julia, JuliaSyntax versions Latest julia works. Changes are needed to work with the latest JuliaSyntax, but that isn't in base julia yet, and more changes are likely to come. * Fix small bug from #16 so tests pass The change lifted the scope of `note`, so it was being changed in the loop * Changes from code review: const/global lowering Ping me if you'd like this squashed into the original const/global commit! Co-authored-by: Claire Foster <[email protected]> * Remove a special case No longer needed since we no longer put `global` or `local` forms back into the expand_forms machine. Some error messages change slightly as a result. * Changes from code review Co-authored-by: Claire Foster <[email protected]> * Fix + test for assignment in value but not tail position * Disallow `static_parameter` as `valid_ir_argument` See added comment, and discussion at #10 (comment) Co-authored-by: Claire Foster <[email protected]> * Change printing of `K"latestworld"` Parens are nice, but it wasn't consistent. Also make it a leaf (remaining non-leaves are deleted in the next commit.) * Move most `latestworld`s to linearization From the docs: ``` The following statements raise the current world age: 1. An explicit invocation of Core.@latestworld 2. The start of every top-level statement 3. The start of every REPL prompt 4. Any type or struct definition 5. Any method definition 6. Any constant declaration 7. Any global variable declaration (but not a global variable assignment) 8. Any using, import, export or public statement 9. Certain other macros like eval (depends on the macro implementation) ``` This commit handles each case as follows: ``` 1. = 9 2. I'm not sure this actually happens (or needs to happen, unless we're being defensive? Doing it after each world-changing operation should suffice). But if we need it, this would just be emitting once at the beginning of every lowered output. 3. = 2 4. = 6 5. Emit seeing `method` in linearize 6. Emit seeing `constdecl` in linearize 7. Emit seeing `global` or `globaldecl` in linearize 8. We just defer to `eval`, but should probably go in desugaring later - using/import recently became builtin calls, and I haven't updated JL to use them yet. Base._import_using has an expr-based API that may change, and our importpath destructuring is worth keeping. - export and public (special forms) are handled in toplevel.c 9. Done for us ``` Other quirks: - `JuliaLowering.eval_closure_type` calls eval to assign a const, so we still need to deal with that in closure conversion. - The `include` hack isn't mentioned in the docs, but can stay in desugaring. I'm not certain why we don't do the same for non-macro `eval`. --------- Co-authored-by: Claire Foster <[email protected]>
* Update CodeInfo struct and handling Co-authored-by: Claire Foster <[email protected]> * Don't produce raw symbol from globalref This used to implicitly refer to a module-level name, but lowering is now expected to wrap it in a `globalref`. Part of #54772 * Updates to const and global lowering; add K"constdecl"; omit `wrap` #54773, #56713, #57470. Some changes omitted from `expand-decls` and `expand-assignment`. Note that the two-argument IR "const" is K"constdecl", whereas the one-argument K"const" only appears in the AST. Also note that the `wrap` parameter is omitted throughout assignment desugaring. As far as I'm aware, all this plumbing was just to support `const a,b,c = 1,2,3` having `b` and `c` inherit the `const`. TODO: find a better way of doing the same thing (a ScopedValue might be a clean solution; we currently throw an error). The check for `let; const x = 1; end`, (which should throw) is in scope analysis (lisp has it in `compile`). Co-authored-by: Claire Foster <[email protected]> * Add `isdefinedglobal` builtin #54999, #56985 * :global no longer valid_ir_argument; rm `is_defined_nothrow_global` #56746. Also call :slot and :static_parameter valid (for now) * Fix `is_defined_and_owned_global` (Core.Binding changes) Adapt to bpart changes in #54788 * Struct desugaring: "Undo decision to publish incomplete types..." #56497; Add self-referencing struct shim I have doubts about how long this solution will stay in the base repository, and how complete it is (doesn't seem to work with M1.M2.S), but we are testing for it here. Also change the expected value of a test changed in the same PR. * Emit `latestworld` world age increments For method defs, `latestworld` is produced in desugaring rather than closure conversion for now (our closure conversion doesn't seem to cover the same cases as lisp lowering yet). Covers #56523, #56509, #57299. Also includes changes from #57102 (bpart: Start enforcing minimum world age for const bparts) and #57150 (bpart: Start enforcing min_world for global variable definitions) since the lowering changes from those appear to be amendments to the changes above (missing world age increments). Co-authored-by: Claire Foster <[email protected]> * bpart changes: `Core._typebody!` signature `Core._typebody!` now takes a new "prev" argument, which we don't use yet here. Changes from #57253 * bpart changes: struct desugaring Changes from #57253 (bpart: Fully switch to partitioned semantics). This fixes one failing test and realigns struct desugaring to match lisp for now. Also changed: the expected result of redefining a primitive type (now allowed). * Additional argument in `new_opaque_closure` Fix segfaulting test. Thanks for the TODO * Adapt to different `GeneratedFunctionStub` signature Signature changed in #57230. Thanks @aviatesk for the help! * Fix `public` and `export` As of #57765, `jl_module_public` is no longer exported. Change our runtime to handle it like `public` and `export` like we handle `import` or `using` for now * Fix modules.jl test I believe this was a world age issue * Regenerate IR tests Too many to count. * Update README to known-good julia, JuliaSyntax versions Latest julia works. Changes are needed to work with the latest JuliaSyntax, but that isn't in base julia yet, and more changes are likely to come. * Fix small bug from JuliaLang/JuliaLowering.jl#16 so tests pass The change lifted the scope of `note`, so it was being changed in the loop * Changes from code review: const/global lowering Ping me if you'd like this squashed into the original const/global commit! Co-authored-by: Claire Foster <[email protected]> * Remove a special case No longer needed since we no longer put `global` or `local` forms back into the expand_forms machine. Some error messages change slightly as a result. * Changes from code review Co-authored-by: Claire Foster <[email protected]> * Fix + test for assignment in value but not tail position * Disallow `static_parameter` as `valid_ir_argument` See added comment, and discussion at c42f/JuliaLowering.jl#10 (comment) Co-authored-by: Claire Foster <[email protected]> * Change printing of `K"latestworld"` Parens are nice, but it wasn't consistent. Also make it a leaf (remaining non-leaves are deleted in the next commit.) * Move most `latestworld`s to linearization From the docs: ``` The following statements raise the current world age: 1. An explicit invocation of Core.@latestworld 2. The start of every top-level statement 3. The start of every REPL prompt 4. Any type or struct definition 5. Any method definition 6. Any constant declaration 7. Any global variable declaration (but not a global variable assignment) 8. Any using, import, export or public statement 9. Certain other macros like eval (depends on the macro implementation) ``` This commit handles each case as follows: ``` 1. = 9 2. I'm not sure this actually happens (or needs to happen, unless we're being defensive? Doing it after each world-changing operation should suffice). But if we need it, this would just be emitting once at the beginning of every lowered output. 3. = 2 4. = 6 5. Emit seeing `method` in linearize 6. Emit seeing `constdecl` in linearize 7. Emit seeing `global` or `globaldecl` in linearize 8. We just defer to `eval`, but should probably go in desugaring later - using/import recently became builtin calls, and I haven't updated JL to use them yet. Base._import_using has an expr-based API that may change, and our importpath destructuring is worth keeping. - export and public (special forms) are handled in toplevel.c 9. Done for us ``` Other quirks: - `JuliaLowering.eval_closure_type` calls eval to assign a const, so we still need to deal with that in closure conversion. - The `include` hack isn't mentioned in the docs, but can stay in desugaring. I'm not certain why we don't do the same for non-macro `eval`. --------- Co-authored-by: Claire Foster <[email protected]>
5 participants
This is a prepratory commit for #54654 to change the lowering of
constand typed globals to be compatible with the new semantics.Currently, we lower
const a::T = valto:(which further expands to typed-globals an implicit converts).
This works, because, under the hood, our const declarations are actually assign-once globals. Note however, that this is not syntactically reachable, since we have a parse error for plain
const a:However, this lowering is not atomic with respect to world age. The semantics in #54654 require that the const-ness and the value are established atomically (with respect to world age, potentially on another thread) or undergo invalidation.
To resolve this issue, this PR changes the lowering of
const a::T = valto:where the latter is a special syntax form
Expr(:const, GlobalRef(,:a), :a).A similar change is made to const global declarations, which previously lowered via intrinsic, i.e.
global a::T = vallowered to:This changes the
set_binding_type!to instead be a syntax formExpr(:globaldecl, :a, T). This is not technically required, but we currently do not use intrinsics for world-age affecting side-effects anywhere else in the system. In particular, after #54654, it would be illegal to callset_binding_type!in anything but top-level context. Now, we have discussed in the past that there should potentially be intrinsic functions for global modifications (method table additions, etc), currently only reachable throughCore.eval, but such an intrinsic would require semantics that differ from both the currentset_binding_type!and the new:globaldecl. Using an Expr form here is the most consistent with our current practice for these sort of things elsewhere and accordingly, this PR removes the intrinsic.Note that this PR does not yet change any syntax semantics, although there could in principle be a reordering of side-effects within an expression (e.g. things like
global a::(@isdefined(a) ? Int : Float64)might behave differently after this commit. However, we never defined the order of side effects (which is part of what this is cleaning up, although, I am not formally defining any specific ordering here either - #54654 will do some of that), and that is not a common case, so this PR should be largely considered non-semantic with respect to the syntax change.Also fixes #54787 while we're at it.