Skip to content

Fix incorrect result for Base.invmod(<:Signed, <:Unsigned) #58010

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 5 commits into from
May 8, 2025
Merged

Fix incorrect result for Base.invmod(<:Signed, <:Unsigned) #58010

merged 5 commits into from
May 8, 2025

Conversation

@alexp616
Copy link
Contributor

@alexp616 alexp616 commented Apr 4, 2025

MWE of bug:

julia> Int(invmod(1024, UInt(12289)))
5652

julia> invmod(1024, 12289)
12277

@LilithHafner LilithHafner added bug Indicates an unexpected problem or unintended behavior maths Mathematical functions bugfix This change fixes an existing bug and removed bug Indicates an unexpected problem or unintended behavior labels Apr 6, 2025
@LilithHafner LilithHafner mentioned this pull request Apr 6, 2025
Closed
LilithHafner
LilithHafner reviewed Apr 6, 2025
View reviewed changes
Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

Thanks! This is a good find and a good approach to fixing it!

@fingolfin @LilithHafner
Apply suggestions from code review
5982b93 
Co-authored-by: Lilith Orion Hafner <[email protected]>
fingolfin
fingolfin reviewed May 8, 2025
View reviewed changes
fingolfin
fingolfin reviewed May 8, 2025
View reviewed changes
@fingolfin fingolfin added the merge me PR is reviewed. Merge when all tests are passing label May 8, 2025
@fingolfin
Copy link
Member

Apology for a flutter of changes and for just applying them in the first place, but I think it'd be a shame to let this slide down the pile further :-). I hope this is OK now

@LilithHafner LilithHafner merged commit 492d10a into JuliaLang:master May 8, 2025
6 of 8 checks passed
@LilithHafner LilithHafner removed the merge me PR is reviewed. Merge when all tests are passing label May 8, 2025
charleskawczynski pushed a commit to charleskawczynski/julia that referenced this pull request May 12, 2025
@alexp616 @fingolfin
@LilithHafner @charleskawczynski
Fix incorrect result for Base.invmod(<:Signed, <:Unsigned) (JuliaLang…
e187ccf 
…#58010)

MWE of bug:
```julia
julia> Int(invmod(1024, UInt(12289)))
5652

julia> invmod(1024, 12289)
12277
```

---------

Co-authored-by: Max Horn <[email protected]>
Co-authored-by: Lilith Orion Hafner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fingolfin fingolfin fingolfin left review comments

@LilithHafner LilithHafner LilithHafner left review comments

Assignees

No one assigned

Labels

bugfix This change fixes an existing bug maths Mathematical functions

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alexp616 @fingolfin @LilithHafner