Skip to content

Correctly revise methods defined in external method tables #904

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.

Sign up for GitHub

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

Jump to bottom
Merged
serenity4 merged 23 commits into from
Jul 26, 2025
Merged

Correctly revise methods defined in external method tables #904

serenity4 merged 23 commits into from
Jul 26, 2025

Conversation

@serenity4
Copy link
Collaborator

@serenity4 serenity4 commented Apr 18, 2025 •
edited
Loading

Fixes #646.

Requires:

To do before merging:

  • Test this "in the wild" with some large packages (SPIRV.jl and GPUCompiler-dependent packages)
  • Update documentation, docstrings and comments.

timholy
timholy approved these changes Apr 19, 2025
View reviewed changes
Copy link
Owner

@timholy timholy left a comment

Choose a reason for hiding this comment

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

This looks great. I don't think that #815 offers anything that this doesn't, so this PR could close that one unless you disagree.

@serenity4 serenity4 force-pushed the revise-external-methodtables branch from e69dcb5 to da59589 Compare April 21, 2025 20:22
@serenity4 serenity4 force-pushed the revise-external-methodtables branch from 516a1a0 to 024f45c Compare April 25, 2025 19:47
@serenity4 serenity4 marked this pull request as ready for review June 10, 2025 13:18
@serenity4
Copy link
Collaborator Author

serenity4 commented Jun 10, 2025

I looked into supporting the mapexpr-form of include, and it doesn't strike me that it would require changes to either CodeTracking or LoweredCodeUtils. I'm not done with that, but it's getting a bit involved and I'm a bit low on time these days.

To avoid leaving this PR rotting for too long I think it might be good to move forward with the release of CodeTracking 2.0 and LoweredCodeUtils 4.0 after JuliaDebug/LoweredCodeUtils.jl#125, so we can merge this PR. Then, if the mapexpr support still eventually requires a new breaking release for either, I would hope the disruption to not be too impactful.

@timholy what do you think?

serenity4
serenity4 commented Jun 10, 2025
View reviewed changes
@serenity4 serenity4 force-pushed the revise-external-methodtables branch from ab8a38e to d302445 Compare June 10, 2025 13:37
@serenity4 serenity4 mentioned this pull request Jun 10, 2025
Merged
@timholy timholy mentioned this pull request Jul 24, 2025
Open
@serenity4
Copy link
Collaborator Author

serenity4 commented Jul 26, 2025

Once LoweredCodeUtils v3.4.3 is available in the registry (JuliaRegistries/General#135445), if all checks pass this is good to merge!

@serenity4 serenity4 merged commit e89f508 into timholy:master Jul 26, 2025
10 of 22 checks passed
This was referenced Jul 26, 2025
Closed
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timholy timholy timholy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Revising methods with external method tables fails

2 participants

@serenity4 @timholy