-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
gf: allow method shadowing with exact signatures without deletion #53415
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vchuravy
reviewed
Feb 21, 2024
We already allowed method shadowing for an inexact signature and implicit method deletion by exact signatures, so it was fairly arbitrary to implement it as a deletion for a replacement by an exact signature rather than use the morespecific definition for this. This change should be useful to Mocking libraries, such as Pluto, which previously had a buggier version of this which tried to re-activate or re-define the old methods. There is a bit of code cleanup in here, either of unused features, or of aspects that were broken, or of world_valid computations that were not actually impacting the final result (e.g. when there is a more specific result matches, it does not need to limit the result valid world range because of a less specific result that does not get returned).
7b21ea3 to
d610327
Compare
tecosaur
pushed a commit
to tecosaur/julia
that referenced
this pull request
Mar 4, 2024
…liaLang#53415) We already allowed method shadowing for an inexact signature and implicit method deletion by exact signatures, so it was fairly arbitrary to implement it as a deletion for a replacement by an exact signature rather than use the morespecific definition for this. This change should be useful to Mocking libraries, such as Pluto, which previously had a buggier version of this which tried to re-activate or re-define the old methods. There is a bit of code cleanup in here, either of unused features, or of aspects that were broken, or of world_valid computations that were not actually impacting the final result (e.g. when there is a more specific result matches, it does not need to limit the result valid world range because of a less specific result that does not get returned).
This was referenced Mar 5, 2024
|
Yayy thanks! I'm really happy that you helped make our method functionality more stable, and also making it easier for other projects in the future! 🌟 |
|
Thanks for inspiring it, basically by having Pluto show that it should be valid and that it is useful |
mkitti
pushed a commit
to mkitti/julia
that referenced
this pull request
Mar 7, 2024
…liaLang#53415) We already allowed method shadowing for an inexact signature and implicit method deletion by exact signatures, so it was fairly arbitrary to implement it as a deletion for a replacement by an exact signature rather than use the morespecific definition for this. This change should be useful to Mocking libraries, such as Pluto, which previously had a buggier version of this which tried to re-activate or re-define the old methods. There is a bit of code cleanup in here, either of unused features, or of aspects that were broken, or of world_valid computations that were not actually impacting the final result (e.g. when there is a more specific result matches, it does not need to limit the result valid world range because of a less specific result that does not get returned).
vtjnash
added a commit
that referenced
this pull request
Apr 30, 2025
…ch_status enum The original purpose of this was to manage quickly detecting if a method was replaced, but that stopped being correct after #53415. It is also a fairly heavy-weight description of a single bit of information, and apparently (despite not having "min" or "max" anywhere in the name), its existence misleads people into thinking that primary/deleted are somehow corresponding to min/max. That is always false: this bit only exists for invalidations, and primary_world is primarily valid to use for supporting generated functions (or cases like `show` where correctness isn't especially important, though we may use it at times as a short-cut to computing the result of a dispatch result). This implements METHOD_ISMINMAX_INVOKE_LATEST fully, and provides just a stub for adding METHOD_ISMINMAX_CALL_LATEST later. Fix #58215
vtjnash
added a commit
that referenced
this pull request
May 6, 2025
…ch_status enum The original purpose of this was to manage quickly detecting if a method was replaced, but that stopped being correct after #53415. It is also a fairly heavy-weight description of a single bit of information, and apparently (despite not having "min" or "max" anywhere in the name), its existence misleads people into thinking that primary/deleted are somehow corresponding to min/max. That is always false: this bit only exists for invalidations, and primary_world is primarily valid to use for supporting generated functions (or cases like `show` where correctness isn't especially important, though we may use it at times as a short-cut to computing the result of a dispatch result). This implements METHOD_ISMINMAX_INVOKE_LATEST fully, and provides just a stub for adding METHOD_ISMINMAX_CALL_LATEST later. Fix #58215
vtjnash
added a commit
that referenced
this pull request
May 6, 2025
…ch_status enum The original purpose of this was to manage quickly detecting if a method was replaced, but that stopped being correct after #53415. It is also a fairly heavy-weight description of a single bit of information, and apparently (despite not having "min" or "max" anywhere in the name), its existence misleads people into thinking that primary/deleted are somehow corresponding to min/max. That is always false: this bit only exists for invalidations, and primary_world is primarily valid to use for supporting generated functions (or cases like `show` where correctness isn't especially important, though we may use it at times as a short-cut to computing the result of a dispatch result). This implements METHOD_ISMINMAX_INVOKE_LATEST fully, and provides just a stub for adding METHOD_ISMINMAX_CALL_LATEST later. Fix #58215
vtjnash
added a commit
that referenced
this pull request
May 6, 2025
…ch_status enum (#58291) The original purpose of this field was to manage quickly detecting if a method was replaced, but that stopped being correct after #53415. It was a fairly heavy-weight description of that single bit of information. This bit of information allows quickly bypassing some method lookups from pkgimages, since it can quickly detect that the result is trivially correct (such as single-argument functions). Also fixes #58215
serenity4
pushed a commit
to serenity4/julia
that referenced
this pull request
May 7, 2025
…ch_status enum (JuliaLang#58291) The original purpose of this field was to manage quickly detecting if a method was replaced, but that stopped being correct after JuliaLang#53415. It was a fairly heavy-weight description of that single bit of information. This bit of information allows quickly bypassing some method lookups from pkgimages, since it can quickly detect that the result is trivially correct (such as single-argument functions). Also fixes JuliaLang#58215
KristofferC
pushed a commit
that referenced
this pull request
May 9, 2025
…ch_status enum (#58291) The original purpose of this field was to manage quickly detecting if a method was replaced, but that stopped being correct after #53415. It was a fairly heavy-weight description of that single bit of information. This bit of information allows quickly bypassing some method lookups from pkgimages, since it can quickly detect that the result is trivially correct (such as single-argument functions). Also fixes #58215 (cherry picked from commit 5eb5155)
KristofferC
pushed a commit
that referenced
this pull request
May 12, 2025
…ch_status enum (#58291) The original purpose of this field was to manage quickly detecting if a method was replaced, but that stopped being correct after #53415. It was a fairly heavy-weight description of that single bit of information. This bit of information allows quickly bypassing some method lookups from pkgimages, since it can quickly detect that the result is trivially correct (such as single-argument functions). Also fixes #58215 (cherry picked from commit 5eb5155)
charleskawczynski
pushed a commit
to charleskawczynski/julia
that referenced
this pull request
May 12, 2025
…ch_status enum (JuliaLang#58291) The original purpose of this field was to manage quickly detecting if a method was replaced, but that stopped being correct after JuliaLang#53415. It was a fairly heavy-weight description of that single bit of information. This bit of information allows quickly bypassing some method lookups from pkgimages, since it can quickly detect that the result is trivially correct (such as single-argument functions). Also fixes JuliaLang#58215
charleskawczynski
pushed a commit
to charleskawczynski/julia
that referenced
this pull request
May 12, 2025
…ch_status enum (JuliaLang#58291) The original purpose of this field was to manage quickly detecting if a method was replaced, but that stopped being correct after JuliaLang#53415. It was a fairly heavy-weight description of that single bit of information. This bit of information allows quickly bypassing some method lookups from pkgimages, since it can quickly detect that the result is trivially correct (such as single-argument functions). Also fixes JuliaLang#58215
KristofferC
pushed a commit
to DilumAluthgeBot/julia
that referenced
this pull request
May 12, 2025
…ch_status enum (JuliaLang#58291) The original purpose of this field was to manage quickly detecting if a method was replaced, but that stopped being correct after JuliaLang#53415. It was a fairly heavy-weight description of that single bit of information. This bit of information allows quickly bypassing some method lookups from pkgimages, since it can quickly detect that the result is trivially correct (such as single-argument functions). Also fixes JuliaLang#58215 (cherry picked from commit 5eb5155)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We already allowed method shadowing for an inexact signature and implicit method deletion by exact signatures, so it was fairly arbitrary to implement it as a deletion for a replacement by an exact signature rather than use the morespecific definition for this.
This change should be useful to Mocking libraries, such as Pluto, which previously had a buggier version of this which tried to re-activate or re-define the old methods.
There is a bit of code cleanup in here, either of unused features, or of aspects that were broken, or of world_valid computations that were not actually impacting the final result (e.g. when there is a more specific result matches, it does not need to limit the result valid world range because of a less specific result that does not get returned).