Skip to content

Conversation

Kivooeo
Copy link
Member

@Kivooeo Kivooeo commented Sep 6, 2025

This fixes very old fixme that sounds like this

Stupid slow base-2 long division taken from
https://en.wikipedia.org/wiki/Division_algorithm
FIXME use a greater base ($ty) for the long division.

By deleting this method since it was never used

@rustbot
Copy link
Collaborator

rustbot commented Sep 6, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 6, 2025
@Mark-Simulacrum
Copy link
Member

Is there actually any usage of this div_rem? A search locally only finds usage in coretests.

Maybe we can just delete it?

$ rg -w div_rem library
library/core/src/num/bignum.rs
341:            pub fn div_rem(&self, d: &$name, q: &mut $name, r: &mut $name) {

library/coretests/tests/num/bignum.rs
172:    fn div_rem(n: u64, d: u64) -> (Big, Big) {
175:        Big::from_u64(n).div_rem(&Big::from_u64(d), &mut q, &mut r);
178:    assert_eq!(div_rem(1, 1), (Big::from_small(1), Big::from_small(0)));
179:    assert_eq!(div_rem(4, 3), (Big::from_small(1), Big::from_small(1)));
180:    assert_eq!(div_rem(1, 7), (Big::from_small(0), Big::from_small(1)));
181:    assert_eq!(div_rem(45, 9), (Big::from_small(5), Big::from_small(0)));
182:    assert_eq!(div_rem(103, 9), (Big::from_small(11), Big::from_small(4)));
183:    assert_eq!(div_rem(123456, 77), (Big::from_u64(1603), Big::from_small(25)));
184:    assert_eq!(div_rem(0xffff, 1), (Big::from_u64(0xffff), Big::from_small(0)));
185:    assert_eq!(div_rem(0xeeee, 0xffff), (Big::from_small(0), Big::from_u64(0xeeee)));
186:    assert_eq!(div_rem(2_000_000, 2), (Big::from_u64(1_000_000), Big::from_u64(0)));

@Kivooeo
Copy link
Member Author

Kivooeo commented Sep 7, 2025

Would it be reasonable to check if other methods were ever used and delete them as well?

But, looking at this from the other side, what if someone were to write something in dec2flt or flt2dec and needed div_rem, but there was nothing?

@Mark-Simulacrum
Copy link
Member

I'm not particularly worried about someone needing it and needing to write it (or dig it up in git history). I definitely don't want to spend reviewer time reviewing optimizations for dead code though.

@Kivooeo
Copy link
Member Author

Kivooeo commented Sep 7, 2025

Fair enough, then first question about other unused methods is still open, and I'm fine to delete them as well if found any other

@Mark-Simulacrum
Copy link
Member

I don't have any strong opinion on whether to proactively audit for other unused code. It's probably relatively harmless to keep it and just not touch it.

@rust-cloud-vms rust-cloud-vms bot force-pushed the blazing-fast-division-bignum branch from bddb431 to a947e5e Compare September 7, 2025 17:21
@rust-cloud-vms rust-cloud-vms bot force-pushed the blazing-fast-division-bignum branch from a947e5e to a2d66db Compare September 7, 2025 17:22
@Kivooeo Kivooeo changed the title Update div_rem algorithm in core::num::bignum Remove div_rem from core::num::bignum Sep 7, 2025
@Kivooeo
Copy link
Member Author

Kivooeo commented Sep 7, 2025

Oh, well

It's probably relatively harmless to keep it and just not touch it.

Do you mean it's better not to touch this function or any others? If first so feel free to close this one I guess

@Mark-Simulacrum
Copy link
Member

No strong opinion either way, changes that are just deleting (dead code) lines I'm generally happy to rubber stamp. They're easy to add back later.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 14, 2025

📌 Commit a2d66db has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 14, 2025
bors added a commit that referenced this pull request Sep 15, 2025
Rollup of 8 pull requests

Successful merges:

 - #143314 (add reference id to test, and fix filename)
 - #146284 (Remove `div_rem` from `core::num::bignum`)
 - #146416 (Tidy dependency checks cleanups + QoL)
 - #146471 (bootstrap: Show target in "No such target exists" message)
 - #146478 (Improve `core::fmt` coverage)
 - #146480 (tests: update new test to accept new lifetime format)
 - #146488 (Improve `core::ptr` coverage)
 - #146501 (compiletest: Fix `--exact` test filtering)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fa63dbf into rust-lang:master Sep 15, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 15, 2025
rust-timer added a commit that referenced this pull request Sep 15, 2025
Rollup merge of #146284 - Kivooeo:blazing-fast-division-bignum, r=Mark-Simulacrum

Remove `div_rem` from `core::num::bignum`

This fixes very old fixme that sounds like this

```
Stupid slow base-2 long division taken from
https://en.wikipedia.org/wiki/Division_algorithm
FIXME use a greater base ($ty) for the long division.
```

By deleting this method since it was never used
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants