Skip to content

Conversation

@mlechu
Copy link
Collaborator

@mlechu mlechu commented Apr 2, 2025

The goal of this change is to have JuliaLowering work on julia nightly again,
particularly for the JETLS development that's going on, by porting recent
changes from julia-syntax.scm. It will be an unfortunately large diff, but a
smaller PR would leave JuliaLowering broken for all versions of julia, so I've
settled on keeping the commit history readable.

Major changes

  • The CodeInfo and Core.Binding structs have been changed
  • Raw symbols from lowering are no longer resolved to globals; wrap them in
    globalref
  • Use new IR forms:
    • Lowering is expected to produce world-age increments (latestworld)
    • globaldecl
      • The plain global form is still emitted from both lowering implementations, but may not be in the future
    • constdecl IR form that takes two arguments like =, not to be confused with the one-argument const AST form
  • New isdefinedglobal builtin
  • signature changes to ::GeneratedFunctionStub and Core._typebody!

TODO

For a future PR:

Commits considered

This list was generated so I could keep track of my work, but it might be helpful here to list bugfixes we don't have yet. Note that it only includes changes to julia-syntax.scm, which doesn't cover all breaking commits. N/A means I intentionally skipped it for some reason (e.g. already done before this PR).

Long list
  • [N/A] b7e72322b3 Don't let setglobal! implicitly create bindings (#54678)
  • [N/A] 1c59231b2d opaque_closure: Allow opting out of PartialOpaque (#54734)
  • 1cd47c3094 lowering: Remove outerref intermediate form (#54772)
  • 67c9989e83 Fix lowering for export and similar (#54812)
  • dfd1d490d5 lowering: Refactor lowering for const and typed globals (#54773)
  • 696d9c3e69 Follow up #54772 - don't accidentally put Module into method name slot (#54856)
  • [N/A] a5f0016008 Support @opaque Tuple{T,U...}->RT (...)->... syntax for explicit arg/return types (#54947)
  • aa0758594f lowering: Don't resolve type bindings earlier than necessary (#54999)
  • 4f1af7f39d Allow opting out of PartialOpaque support via Expr(:opaque_closure, ...) (#55068)
  • [N/A] 0c9c1e25b7 Canonicalize names of nested functions by keeping a more fine grained counter -- per (module, method name) pair (#53719)
  • 2616634a17 fix #45494, error in ssa conversion with complex type decl (#55744)
  • [N/A] 911e02558d better error for esc outside of macro expansion (#55797)
  • dc344285d5 Fix type instability of closures capturing types (2) (#40985)
  • 6de6b46b7e lowering: split finally blocks for exceptional control-flow (#55876)
  • 435516da3a Undo the decision to publish incomplete types to the binding table (#56497)
  • 505907bd11 Add lowering and interpreter support for :latestworld (#56523)
  • deac82ad91 lowering: don't reverse handler order in (pop-handler-list ...) (#55871)
  • 034e6093c5 Make world-age increments explicit (#56509)
  • f1b0b010dd Fix scope of hoisted signature-local variables (#56712)
  • 2c87290f2e lowering: Canonicalize to builtins for global assignment (#56713)
  • 3a68b035cc Fully outline all GlobalRefs (#56746)
  • 3d85309e80 Consolidate various isdefined functionality into a new builtin (#56985)
  • 30177d0578 cleanup: Remove fallback post-lowering global resolution (#57051)
  • 7f99e95377 bpart: Start enforcing minimum world age for const bparts (#57102)
  • f209eba244 bpart: Start enforcing min_world for global variable definitions (#57150)
  • 888cf03506 bpart: Fully switch to partitioned semantics (#57253)
  • e553fc8c6f Add missing latestworld after parameterized type alias (#57299)
  • 512eb5e2c9 lowering: Only try to define the method once (#57346)
  • 91e733384e internals: add _defaultctor function for defining ctors
  • 414aca21e9 lowering: Don't mutate lambda in linearize (#57416)
  • 3c02af98dc lowering: Handle malformed ... expressions (#57480)
  • 2e57730aa2 lowering: Allow chaining of >: in where (#57554)
  • 7fa0c13b93 Make no-body function declaration implicitly global (#57562)
  • ca17927311 lowering: Don't closure-convert in import or using (#57774)
  • 6043569ffd Factor out expand-table '= lambda into expand-assignment
  • 9a437a52d3 const lowering: respect scope, prohibit non-const const assignment
  • 64672f5e8a const lowering: resolve scopes under assign-const-if-global (#57470)
  • 3360a44a4d Disallow non-lhs all-underscore variable names (#57626)
  • 1570bed513 Align :method Expr return value between interpreter and codegen (#58076)

@mlechu
Copy link
Collaborator Author

mlechu commented Apr 18, 2025

All tests (excluding IR equality tests) are passing, so I'm marking this as ready for review!

@mlechu mlechu changed the title WIP: Compatibility with julia nightly Compatibility with julia nightly Apr 18, 2025
mlechu added a commit to JuliaLang/JuliaSyntax.jl that referenced this pull request Apr 23, 2025
Moved to [JuliaLowering](c42f/JuliaLowering.jl#10)
     instead (the data structure we need is defined there).
mlechu added a commit to JuliaLang/JuliaSyntax.jl that referenced this pull request Apr 23, 2025
Moved to [JuliaLowering](c42f/JuliaLowering.jl#10)
     instead (the data structure we need is defined there).
@c42f
Copy link
Owner

c42f commented Apr 29, 2025

This is pretty epic, thanks!

Doing this port was going to be my first task back to get me back into the swing of things 😆

Regarding the TODOs - definitely best to do them later, except for the tests. We should keep the IR tests working.

I'm keen on moving to the use of utility functions like _defaultctors where possible to keep the IR cleaner and more compact. In fact JuliaLowering has several such runtime functions in src/runtime.jl already. For example, eval_closure_type() is almost entirely analogous. This can be done in a separate PR though as it reduces churn in the IR tests.

I think we should have K"constdecl" or some such instead of having const be semantically ambiguous. We can translate that to the runtime's const-with-nested-GlobalRef form at the last stage of lowering.

@mlechu
Copy link
Collaborator Author

mlechu commented Apr 29, 2025

We should keep the IR tests working.

Sure, will update them

I think we should have K"constdecl" or some such instead of having const be semantically ambiguous. We can translate that to the runtime's const-with-nested-GlobalRef form at the last stage of lowering.

Already done (albeit buried), see aa32c09. I didn't do this at first because I thought both const forms were the same 🙃. Note that this form is probably getting eliminated once JuliaLang/julia#58187 is merged.

@c42f
Copy link
Owner

c42f commented Apr 30, 2025

Already done

Amazing, thanks! I did see this as soon as I started to do an in depth review.

For the tests, you can use the utility function in test/util.jl refresh_all_ir_tests() I think it's called.

In addition to existing tests, please add new tests if there's new features to cover. I imagine latestworld could do with some tests to cover the specific conditions like explicit include() - I remember there was some slightly hacky conditions keno was forced to add for compat?

Can you explain the wrap stuff a bit? I vaguely recall deciding with @JeffBezanson that we would try to avoid going to heroic lengths to deal with this edge case, which iirc is arguably a miss-feature of Julia's syntax. But I can't remember the details, it could have been something else!

@c42f
Copy link
Owner

c42f commented Apr 30, 2025

Generally, I'll note that I'm taking the occasional liberty with compatibility when I really feel some obscure feature is poorly motivated in modern Julia. Especially when it's an "emergent feature" of the implementation, rather than something done purposefully.

There's not many such cases and it's still always a really hard call, but I feel a brand new implementation does provide a little extra freedom here, especially when the code complexity of implementing a mis-feature is high!

@mlechu
Copy link
Collaborator Author

mlechu commented Apr 30, 2025

please add new tests if there's new features to cover

Will do. Once we can use JuliaLowering in core JuliaLang/julia#58207, we can also try using some of the lowering tests from the julia repository without huge manual porting effort.

I vaguely recall deciding with @ JeffBezanson that we would try to avoid going to heroic lengths to deal with this edge case

I'm pretty happy to delete wrap haha. I think I overestimated the level of compatibility with lisp lowering you were going for. I agree this is a good opportunity to let old emergent behaviour go.

@mlechu
Copy link
Collaborator Author

mlechu commented May 1, 2025

OK, this is ready for more review. wrap turned out to be necessary for bringing const into destructuring assignments, so I chose to make const [tuple] = ... throw a LoweringError over having it silently drop the const for now—it's probably worth exploring other ways of making that work

topolarity added a commit to JuliaLang/julia that referenced this pull request May 7, 2025
This PR along with [the associated JuliaLowering
PR](c42f/JuliaLowering.jl#10) allow us to
experiment
with JuliaLowering as the core lowering implementation, mainly for JETLS
development at the moment (cc @aviatesk, @c42f).

## Changes

JuliaLowering works with `JuliaLowering.SyntaxTree` input instead of
Expr. This
change allows`SyntaxTree`s out of parsing and into lowering, hopefully
without
disturbing existing code expecting Expr.  The current design wraps
`x::SyntaxTree` like `Expr(:syntaxtree, x)` because otherwise
`toplevel_eval_flex` wouldn't know what `x` is, but once we're past the
experimental phase, it would only make sense for the compiler to
actually know
about the types it's operating on.

### New: `Core._lower`

It (a julia entrypoint), `jl_lower` (C entrypoint), `jl_fl_lower`, and
`fl_lower` all mirror the existing parsing API which was added in
#35243.


<details>

<summary>
Click for pre-PR call graph
</summary>


![graph(6)](https://github.com/user-attachments/assets/a993eb92-5668-4ee7-837f-801feb686cb4)

</details>

After this change:


![graph(7)](https://github.com/user-attachments/assets/697b93cf-b3df-47b2-bc0e-c358fb0a6dca)

Like `Core._parse`, `Core._lower` returns an `svec` with the first
element as
the expected result, and the remaining elements can hold extra
information like
the lowered SyntaxTree with provenance info (I've left the specification
of
extra return values for later, when JuliaLowering is more stable and
JETLS knows
what it needs, because we may opt to pack provenance into the lowered
CodeInfo
object instead). Parsing only uses the svec to return an offset, but it
could
actually make use of the the flexibility to return diagnostics in the
future.

It's unfortunately not clear what types we can expect to go into and
come out of
lowering (I got Expr, LineNumberNode, String, Symbol, and Nothing in
testing.)

### Remove `jl_lower_expr_mod`

I realize this isn't being called from C, and Core._lower is a more
sensible
entry point from julia. We could probably also remove some uncalled
parse and
jl_load functions.

## Limitations

These belong to JuliaLowering, but are listed here as known issues for
anyone
who wants to try it out.

- `(module ...)` forms don't go through JuliaLowering yet. Modules are
lowered
to a call to `JuliaLowering.eval_module(parentmod, modname,
syntaxtree)`,
which is currently a stub function that converts to Expr for lowering
and
  evaluation, which JuliaLowering can't use.
- Macros work differently.  Defining and calling macros should work, but
  invoking normal macros defined pre-JuliaLowering will not.
- Compilation is currently slow (~30 seconds for me).
@mlechu
Copy link
Collaborator Author

mlechu commented May 15, 2025

A couple of opaque closure and generated function tests are re-broken on nightly, lol. I will take a look.

Edit: Generated function IR tests just need to stay in sync with JuliaSyntax changes (I'm using JuliaLang/JuliaSyntax.jl#552, which hasn't been released). Opaque closures hit assertion failures (unrelated to this change) in jl_make_opaque_closure_method, but work most of the time on non-debug builds somehow.

@Keno
Copy link
Contributor

Keno commented May 21, 2025

If this gets us closer to master can we merge this and then work on the remaining points after? I'd like to start playing with this so that we can have a concrete plan for 1.13.

aviatesk added a commit to aviatesk/JETLS.jl that referenced this pull request Jun 14, 2025
Use the `aviatesk/JuliaLowering.jl#avi/JETLS` branch which
combines the following JL branches:
- [`mlechu:fix-nightly`](c42f/JuliaLowering.jl#10)
- [`aviatesk:avi/precompile`](c42f/JuliaLowering.jl#14)

This is a temporary solution until these branches are merged
into JL#master.

This enables JL precompilation and significantly improves
JETLS startup time (even when JETLS `precompile_workload` is
set to `false`).
aviatesk added a commit to aviatesk/JETLS.jl that referenced this pull request Jun 14, 2025
Use the `aviatesk/JuliaLowering.jl#avi/JETLS` branch which
combines the following JL branches:
- [`mlechu:fix-nightly`](c42f/JuliaLowering.jl#10)
- [`aviatesk:avi/precompile`](c42f/JuliaLowering.jl#14)

This is a temporary solution until these branches are merged
into JL#master.

This enables JL precompilation and significantly improves
JETLS startup time (even when JETLS `precompile_workload` is
set to `false`).
Copy link
Owner

@c42f c42f left a 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 try to get through this commit by commit.

First, for CodeInfo changes - looks great though I think we need to set one of the new fields.

(There's also another couple of comments which were sitting around from last time I started going through this - sorry! I think I'll just post them, though)

@mlechu mlechu force-pushed the fix-nightly branch 2 times, most recently from 8b9cd53 to d180775 Compare July 29, 2025 22:08
Copy link
Owner

@c42f c42f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to review the part of this related to "Updates to const and global lowering" ... I'm not sure I got all the commits related to this, though.

mlechu and others added 11 commits August 1, 2025 17:08
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).
Fix segfaulting test.  Thanks for the TODO
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
I believe this was a world age issue
Too many to count.
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.
The change lifted the scope of `note`, so it was being changed in the loop
Ping me if you'd like this squashed into the original const/global commit!

Co-authored-by: Claire Foster <[email protected]>
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.
Co-authored-by: Claire Foster <[email protected]>
@c42f
Copy link
Owner

c42f commented Aug 2, 2025

Thanks for working towards making the inputs and outputs of each pass understandable. On this note, why do we talk about K"BindingId" in desugaring?

Thanks! I'm sure there's more we can do here 😊

Ok, so K"BindingId" can be emitted by desugaring. For example, in the new system SSA vars are just represented as K"BindingId" with is_ssa set to true. There's also cases where we emit mutable bindings from desugaring.

There's admittedly some tension in this - bindings don't obey the scoping rules of the surface language, so the scope resolution pass has no effect on them. From this point of view, it's tempting to use normal K"Identifier" inside desugaring, along with the scope layer system to ensure uniqueness of identifiers introduced there. Thus desugaring would really just be builtin macro expansion. This is conceptually clean and quite satisfying in some ways, but it comes with some practical downsides

  • We'd need to a way to store metadata which is currently very simply handled by Binding fields such as is_ssa and is_internal, etc.
  • For identifiers which would currently be SSA vars we don't want the scope resolution rules to run. Or if they do run, all results will just redo information we already knew in desugaring

It could be worth revisiting this at some stage. It's not clear whether there would be practical benefits in lowering itself (and there would be some amount of performance cost), though clarifying the role of each pass and separating them cleanly is good in general.

@mlechu
Copy link
Collaborator Author

mlechu commented Aug 4, 2025

in the new system SSA vars are just represented as K"BindingId" with is_ssa set to true

This makes sense, thanks. My gut tells me it wouldn't be worth changing for the sake of tidiness, but that having bindings not show up in desugaring input could serve as a hint that some other refactor is on the right track. (My dream is to eventually stop feeding so much expand-forms output back into expand-forms.)

See added comment, and discussion at
    c42f#10 (comment)

Co-authored-by: Claire Foster <[email protected]>
7 (call core.apply_type %%%₆)
8 (call core.UnionAll %%₇)
9 (= slot₁/X %₈)
10 latestworld
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flisp lowering emits this, but I don't think it should.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems correct! This is a good sign for the new implementation :-)

@mlechu mlechu force-pushed the fix-nightly branch 3 times, most recently from 84a55b8 to 62a45c6 Compare August 4, 2025 21:38
mlechu added 2 commits August 4, 2025 15:02
Parens are nice, but it wasn't consistent.

Also make it a leaf (remaining non-leaves are deleted in the next commit.)
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`.
Copy link
Owner

@c42f c42f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking pretty good now, let's get it merged :)

@mlechu mlechu merged commit f4b6df3 into c42f:main Aug 4, 2025
1 of 3 checks passed
c42f added a commit to JuliaLang/julia that referenced this pull request Oct 17, 2025
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants