Skip to content

Conversation

@aviatesk
Copy link
Member

Changes MethodInfoKey from a Pair type alias to a proper struct, avoiding type piracy that would be introduced by defining Pair{...}(::Method) conversion methods. The struct implements Base.iterate to maintain compatibility with destructuring patterns.

This is a breaking change as it changes the representation of method info keys, which packages like LCU and Revise depend heavily on. So we unfotunately need to bump the major version from 2 to 3.

As an alternative, removing Pair{...}(::Method) would also be possible, but that would also break LCU and Revise, so I'm taking this approach which appears to be a better and moer complete fix.

Changes `MethodInfoKey` from a `Pair` type alias to a proper struct,
avoiding type piracy that would be introduced by defining
`Pair{...}(::Method)` conversion methods. The struct implements
`Base.iterate` to maintain compatibility with destructuring patterns.

This is a breaking change as it changes the representation of method
info keys, which packages like LCU and Revise depend heavily on. So we
unfotunately need to bump the major version from 2 to 3.

As an alternative, removing `Pair{...}(::Method)` would also be
possible, but that would also break LCU and Revise, so I'm taking this
approach which appears to be a better and moer complete fix.
@aviatesk aviatesk requested a review from serenity4 November 12, 2025 18:03
Copy link
Collaborator

@serenity4 serenity4 left a comment

Choose a reason for hiding this comment

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

Sounds good to me!

I'll leave it to you and @timholy to decide whether this change is desired enough to justify introducing a new breaking release. I don't really have an opinion on that myself.

@aviatesk
Copy link
Member Author

If there are no objections, I would like to proceed with this PR in the direction of releasing a new major version (this is needed for JETLS to use Revise for development, which will use a vendored CodeTracking implementation internally and would cause precompilatio warning without this change: aviatesk/JETLS.jl#314).

@timholy Afraid to ping you, but please let me know if you have any concerns. If there's no response, I will take responsibility for moving this PR forward while updating the Revise ecosystem myself.

@aviatesk aviatesk merged commit 3819238 into master Nov 26, 2025
16 of 24 checks passed
@aviatesk aviatesk deleted the avi/avoid-MethodInfoKey-Pair branch November 26, 2025 15:27
@timholy
Copy link
Member

timholy commented Dec 18, 2025

Sorry I didn't notice this. It was fine to merge this. Feel free to ping me on slack any time. (There are plenty of times where my other responsibilities make it unrealistic to spend the ~40min per day it requires to stay on top of my notifications.)

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