Skip to content
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

make Cthulhu use Compiler.jl as stdlib optionally when it's loaded #619

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aviatesk
Copy link
Member

Using the Compiler stdlib does make things a bit tricky because we have
to consider Compiler.jl's compatibility not just with the package itself,
but also with the runtime system. In the past, we only had to worry
about the former, so issues didn't come up as often (since using
Core.Compiler guarantees that the compiler is compatible with the
runtime system always).

This commit changes Cthulhu back to using Base.Compiler until the user
specifically loads the Compiler.jl standard library.
This should reduce how often Cthulhu breaks (in normal use).

When an user loads Compiler explicitly, then Cthulhu now load the
CthulhuCompilerExt extension module, and wraps the implementations of
exported APIs.

@aviatesk aviatesk requested a review from Keno February 19, 2025 12:01
@aviatesk aviatesk force-pushed the avi/refactor branch 2 times, most recently from 9483dd1 to d14a761 Compare February 19, 2025 18:06
@aviatesk
Copy link
Member Author

Alternatively, it might be simpler to keep Compiler.jl as a regular (not used by default) dependency in Project.toml and switch Cthulhu.CC to Base.Compiler or the stdlib Compiler based on the settings in Preferences.jl. Any thought on this?

@vchuravy
Copy link
Member

Alternatively, it might be simpler to keep Compiler.jl as a regular (not used by default) dependency in Project.toml and switch Cthulhu.CC to Base.Compiler or the stdlib Compiler based on the settings in Preferences.jl. Any thought on this?

I would do it the opposite? Use Compiler as default and Base.Compiler when a preference asks for it?
I.e. the normal case of using Cthulhu when not using julia#main should be the priority.

@aviatesk
Copy link
Member Author

Alternatively, it might be simpler to keep Compiler.jl as a regular (not used by default) dependency in Project.toml and switch Cthulhu.CC to Base.Compiler or the stdlib Compiler based on the settings in Preferences.jl. Any thought on this?

I would do it the opposite? Use Compiler as default and Base.Compiler when a preference asks for it? I.e. the normal case of using Cthulhu when not using julia#main should be the priority.

The problem is we don't have clear rules for releasing the Compiler.jl stdlib. So we often find situations where regular users not using julia#master (like those on v1.12 nightly) end up using a Compiler.jl stdlib incompatible with v1.12 when they use Cthulhu, since the compiler's stdlib gets released at random times, based on the #master branch code at that moment, without considering e.g. backports.

So I think the default should still be Base.Compiler.

@vchuravy
Copy link
Member

vchuravy commented Feb 20, 2025

Shouldn't the Compiler lib then declare itself incompatible with 1.12? This would seem to be a general problem?

Ideally we would have a Compiler 1.12 release that is always compatible with 1.12

@aviatesk
Copy link
Member Author

Yeah, managing Compiler.jl versions is a general problem.

We need to discuss and decide how to handle compatible Julia runtime versions, or how to release Compiler.jl versions that support older runtimes.

This pull request is a temporary fix until we set those rules.

Even if this is a temporal solution, we need to decide at least one of the following:

  • Which compiler implementation to load by load, Base.Compiler or the Compiler stdlib.
  • How to switch Compiler implementations: using Preferences.jl, or using a package extension that hooks loading Compiler.jl.

aviatesk added a commit to JuliaLang/julia that referenced this pull request Feb 24, 2025
For external packages like Cthulhu that use the Compiler.jl stdlib, it's
quite a hassle to manage all three types of compatibility: between the
external package, the Julia runtime, and Compiler.jl.

I'm thinking instead of handling these compatibility issues by some
complex version engineering for Compiler.jl, we should keep its use
supplementary. External `AbstractInterpreter` packages should primarily
use `Base.Compiler`, while managing compatibility with the Julia runtime
and `Base.Compiler`. This would bring us back to the previous situation,
but it seems better in terms of compatibility management.

On the other hand, using the Compiler.jl stdlib allows for easy
switching of the `Compiler` implementation, which should be an optional
benefit. This is especially useful when developing the `Compiler`
itself, where it's common to do `pkg> dev /path/to/Compiler`. In such
cases, the implementation of the Compiler.jl stdlib should be used
instead of `Base.Compiler`.
As a mechanism to switch the `Compiler` implementation used by external
`AbstractInterpreter` packages, using package extensions might be the
simplest approach. As shown in
[this example PR](JuliaDebug/Cthulhu.jl#619),
we can switch the `Compiler` implementation when `using Compiler` is
called[^1], and still precompile the usual code. However, to list
Compiler.jl in `[extensions]`, it needs a version. To this end, I
propose releasing Compiler.jl versions in line with the Julia runtime
versions, following other Julia standard libraries. But we would only do
minor version releases, not patch releases, which are harder to define.
Specifically, we could release a corresponding version of Compiler.jl
when a release branch is cut. After that, we wouldn't manage patch
versions, and as long as `pkg> dev /path/to/Compiler` works, this simple
versioning should suffice.
Alternatively, we could register a single version `1.0.0` of Compiler.jl
with a dummy implementation, letting the external package determine if
the implementation is real (developed) or dummy, but this seems more
hacky than using package extensions.

[^1]: Also, I'm aware of another issue: when using the Compiler.jl
  stdlib, you need to run `InteractiveUtils.@activate Compiler` first,
  or else some reflection utilities won't work (for example,
  `Base._which(tt; method_table=Compiler.method_table(myinterp))`).
  I think this problem can be solved by adding
  `InteractiveUtils.@activate` in the `__init__` function of the package
  extension code, but I'm not sure if this is a safe solution. It might
  be better to set up a callback that only gets called when
  `InteractiveUtils.@activate` is executed?
Using the Compiler stdlib does make things a bit tricky because we have
to consider Compiler.jl's compatibility not just with the package itself,
but also with the runtime system. In the past, we only had to worry
about the former, so issues didn't come up as often (since using
`Core.Compiler` guarantees that the compiler is compatible with the
runtime system always).

This commit changes Cthulhu back to using `Base.Compiler` until the user
specifically loads the `Compiler.jl` standard library.
This should reduce how often Cthulhu breaks (in normal use).

When an user loads Compiler explicitly, then Cthulhu now load the
CthulhuCompilerExt extension module, and wraps the implementations of
exported APIs.
aviatesk added a commit to JuliaLang/julia that referenced this pull request Feb 25, 2025
For external packages like Cthulhu that use the Compiler.jl stdlib, it's
quite a hassle to manage all three types of compatibility: between the
external package, the Julia runtime, and Compiler.jl.

I'm thinking instead of handling these compatibility issues by some
complex version engineering for Compiler.jl, we should keep its use
supplementary. External `AbstractInterpreter` packages should primarily
use `Base.Compiler`, while managing compatibility with the Julia runtime
and `Base.Compiler`. This would bring us back to the previous situation,
but it seems better in terms of compatibility management.

On the other hand, using the Compiler.jl stdlib allows for easy
switching of the `Compiler` implementation, which should be an optional
benefit. This is especially useful when developing the `Compiler`
itself, where it's common to do `pkg> dev /path/to/Compiler`. In such
cases, the implementation of the Compiler.jl stdlib should be used
instead of `Base.Compiler`.
As a mechanism to switch the `Compiler` implementation used by external
`AbstractInterpreter` packages, using package extensions might be the
simplest approach. As shown in
[this example PR](JuliaDebug/Cthulhu.jl#619),
we can switch the `Compiler` implementation when `using Compiler` is
called[^1], and still precompile the usual code. However, to list
Compiler.jl in `[extensions]`, it needs a version. To this end, I
propose releasing Compiler.jl versions in line with the Julia runtime
versions, following other Julia standard libraries. But we would only do
minor version releases, not patch releases, which are harder to define.
Specifically, we could release a corresponding version of Compiler.jl
when a release branch is cut. After that, we wouldn't manage patch
versions, and as long as `pkg> dev /path/to/Compiler` works, this simple
versioning should suffice.
Alternatively, we could register a single version `1.0.0` of Compiler.jl
with a dummy implementation, letting the external package determine if
the implementation is real (developed) or dummy, but this seems more
hacky than using package extensions.

[^1]: Also, I'm aware of another issue: when using the Compiler.jl
  stdlib, you need to run `InteractiveUtils.@activate Compiler` first,
  or else some reflection utilities won't work (for example,
  `Base._which(tt; method_table=Compiler.method_table(myinterp))`).
  I think this problem can be solved by adding
  `InteractiveUtils.@activate` in the `__init__` function of the package
  extension code, but I'm not sure if this is a safe solution. It might
  be better to set up a callback that only gets called when
  `InteractiveUtils.@activate` is executed?
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.

2 participants