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 ml_matches return nothing when there are too many matches #44478

Merged
merged 1 commit into from
Nov 29, 2022

Conversation

aviatesk
Copy link
Sponsor Member

@aviatesk aviatesk commented Mar 6, 2022

Continuation from #44448 (comment).
Previously ml_matches (and its dependent utilities like _methods_by_ftype)
returned false for cases when there are too many matching method, but
its consumer like findall(::Type, ::InternalMethodTable) usually handles
such case as missing. This commit does a refactor so that they all use
the more consistent value nothing for representing that case.

base/compiler/methodtable.jl Outdated Show resolved Hide resolved
@aviatesk aviatesk force-pushed the avi/toomanynothing branch 2 times, most recently from 20d11d4 to c85b7c3 Compare March 7, 2022 01:23
base/compiler/bootstrap.jl Outdated Show resolved Hide resolved
Base automatically changed from avi/overlayedfindsup to master March 7, 2022 04:16
Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

Seems right, but @JeffBezanson should also review, probably, since he originally made this return false.

@aviatesk aviatesk force-pushed the avi/toomanynothing branch 2 times, most recently from e8bf652 to 0cbcdbd Compare March 9, 2022 07:42
@aviatesk aviatesk force-pushed the avi/toomanynothing branch 2 times, most recently from 9e14841 to c445ac0 Compare March 16, 2022 03:30
@Liozou
Copy link
Member

Liozou commented Apr 14, 2022

Is there any plan for this? I would like to know before bumping #44434 because of a minor conflict in REPLCompletions.jl.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Nov 28, 2022

bump? this should be good to merge

@aviatesk aviatesk force-pushed the avi/toomanynothing branch 3 times, most recently from 04f6c8f to 9aa1f00 Compare November 28, 2022 23:38
Continuation from <#44448 (comment)>.
Previously `ml_matches` (and its dependent utilities like `_methods_by_ftype`)
returned `false` for cases when there are too many matching method, but
its consumer like `findall(::Type, ::InternalMethodTable)` usually handles
such case as `missing`. This commit does a refactor so that they all use
the more consistent value `nothing` for representing that case.
@aviatesk aviatesk merged commit f6e911a into master Nov 29, 2022
@aviatesk aviatesk deleted the avi/toomanynothing branch November 29, 2022 04:18
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.

None yet

3 participants