Skip to content

fix error show edge case #44319

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
merged 3 commits into from
Mar 3, 2022
Merged

fix error show edge case #44319

merged 3 commits into from
Mar 3, 2022

Conversation

oscardssmith
Copy link
Member

fixes #44289

@oscardssmith oscardssmith added error handling Handling of exceptions by Julia or the user backport 1.8 Change should be backported to release-1.8 bugfix This change fixes an existing bug labels Feb 23, 2022
@Keno Keno added the needs tests Unit tests are required for this change label Feb 23, 2022
@KristofferC KristofferC mentioned this pull request Feb 24, 2022
Merged
47 tasks
@oscardssmith
Copy link
Member Author

Can we merge this without tests? I have a reliable reproducer

julia> eval(Expr(:function, :(f()), 1))
f (generic function with 1 method)

julia> f(1)
ERROR: MethodError: no method matching f(::Int64)┌ Error: Error showing method candidates, aborted
│   exception =
│    could not determine location of method definition
│    Stacktrace:

but the error occurs in error printing which is annoying to test.

@KristofferC
Copy link
Member

KristofferC commented Mar 3, 2022 •
edited
Loading

Hm, why is it annoying to test? I think there is a bunch of tests of error printing. Here is an example #43158. You can also catch the error and check what sprint(showerror, ...) outputs.

@oscardssmith
Copy link
Member Author

TIL about showerror. Thanks! I believe this is now ready to merge.

@oscardssmith oscardssmith added display and printing Aesthetics and correctness of printed representations of objects. and removed needs tests Unit tests are required for this change labels Mar 3, 2022
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Mar 3, 2022
simeonschaub
simeonschaub reviewed Mar 3, 2022
View reviewed changes
@oscardssmith @simeonschaub
Update test/error.jl
113f20b 
Co-authored-by: Simeon Schaub <[email protected]>
@oscardssmith oscardssmith merged commit a9d8c85 into JuliaLang:master Mar 3, 2022
@oscardssmith oscardssmith deleted the fix-show_method_candidates branch March 3, 2022 21:32
@oscardssmith oscardssmith removed the merge me PR is reviewed. Merge when all tests are passing label Mar 3, 2022
@IanButterworth IanButterworth mentioned this pull request Mar 4, 2022
Merged
KristofferC pushed a commit that referenced this pull request Mar 4, 2022
@oscardssmith @simeonschaub @KristofferC
fix error show edge case (#44319)
62e55d7 
* fix errors

Co-authored-by: Simeon Schaub <[email protected]>
(cherry picked from commit a9d8c85)
KristofferC pushed a commit that referenced this pull request Mar 7, 2022
@oscardssmith @simeonschaub @KristofferC
fix error show edge case (#44319)
f2539a0 
* fix errors

Co-authored-by: Simeon Schaub <[email protected]>
(cherry picked from commit a9d8c85)
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simeonschaub simeonschaub simeonschaub left review comments

Assignees
No one assigned
Labels
bugfix This change fixes an existing bug display and printing Aesthetics and correctness of printed representations of objects. error handling Handling of exceptions by Julia or the user
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error determining location should not throw in showing method candidates
5 participants
@oscardssmith @KristofferC @simeonschaub @vtjnash @Keno