Skip to content

Stabilize num_midpoint_signed feature#134340

Merged
bors merged 1 commit intorust-lang:masterfrom
Urgau:stabilize-num_midpoint_signed
Feb 21, 2025
Merged

Stabilize num_midpoint_signed feature#134340
bors merged 1 commit intorust-lang:masterfrom
Urgau:stabilize-num_midpoint_signed

Conversation

@Urgau
Copy link
Member

@Urgau Urgau commented Dec 15, 2024

This PR proposes that we stabilize the signed variants of iN::midpoint, the operation is equivalent to doing (a + b) / 2 in a sufficiently large number.

The stabilized API surface would be:

/// Calculates the middle point of `self` and `rhs`.
///
/// `midpoint(a, b)` is `(a + b) / 2` as if it were performed in a
/// sufficiently-large signed integer type. This implies that the result is
/// always rounded towards zero and that no overflow will ever occur.

impl i{8,16,32,64,128,size} {
    pub const fn midpoint(self, rhs: Self) -> Self;
}

T-libs-api previously stabilized the unsigned (and float) variants in #131784, the signed variants were left out because of the rounding that should be used in case of negative midpoint.

This stabilization proposal proposes that we round towards zero because:

  • it makes the obvious (a + b) / 2 in a sufficiently-large number always true
    • using another rounding for the positive result would be inconsistent with the unsigned variants
  • it makes midpoint(-a, -b) == -midpoint(a, b) always true
  • it is consistent with midpoint(a as f64, b as f64) as i64
  • it makes it possible to always suggest midpoint as a replacement for (a + b) / 2 expressions (which we may want to do as a future work given the 21.2k hits on GitHub Search)

@scottmcm mentioned a drawback in #132191 (comment):

I'm torn, because rounding towards zero makes it "wider" than other values, which >> 1 avoids -- (a + b) >> 1 has the nice behaviour that midpoint(a, b) + 2 == midpoint(a + 2, b + 2).

But I guess overall sticking with (a + b) / 2 makes sense as well, and I do like the negation property 🤷

Which I think is outweigh by the advantages cited above.

Closes #110840
cc @rust-lang/libs-api
cc @scottmcm
r? @dtolnay

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tracking Issue for num_midpoint

8 participants