Skip to content

Add extra unchecked_disjoint_bitor implementations.#147368

Open
bjoernager wants to merge 1 commit into
rust-lang:mainfrom
bjoernager:disjoint-bitor
Open

Add extra unchecked_disjoint_bitor implementations.#147368
bjoernager wants to merge 1 commit into
rust-lang:mainfrom
bjoernager:disjoint-bitor

Conversation

@bjoernager

@bjoernager bjoernager commented Oct 5, 2025 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

View all comments

Tracking issue: #135758.

This PR implements the unchecked_disjoint_bitor method for additional types:

impl bool {
    pub const unsafe fn unchecked_disjoint_bitor(self, rhs: Self) -> Self;
}

impl i8 {
    pub const unsafe fn unchecked_disjoint_bitor(self, rhs: Self) -> Self;
}

impl i16 {
    pub const unsafe fn unchecked_disjoint_bitor(self, rhs: Self) -> Self;
}

impl i32 {
    pub const unsafe fn unchecked_disjoint_bitor(self, rhs: Self) -> Self;
}

impl i64 {
    pub const unsafe fn unchecked_disjoint_bitor(self, rhs: Self) -> Self;
}

impl i128 {
    pub const unsafe fn unchecked_disjoint_bitor(self, rhs: Self) -> Self;
}

impl isize {
    pub const unsafe fn unchecked_disjoint_bitor(self, rhs: Self) -> Self;
}

Our disjoint_bitor intrinsic is already implemented for these types. This method specifically is also already present on unsigned, integral types.

Additionally, this PR rewrites the existing docs for unchecked_disjoint_bitor and adds the must_use attribute to the method.

@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 Oct 5, 2025
@rustbot

rustbot commented Oct 5, 2025

Copy link
Copy Markdown
Collaborator

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

@bjoernager

bjoernager commented Oct 5, 2025

Copy link
Copy Markdown
Contributor Author

@rustbot label +T-libs-api -T-libs

r? libs-api

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 5, 2025
@rustbot rustbot assigned BurntSushi and unassigned Mark-Simulacrum Oct 5, 2025
@bjoernager bjoernager mentioned this pull request Oct 5, 2025
Open
3 tasks
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bjoernager

This comment was marked as outdated.

Sign in to view
@bjoernager

bjoernager commented Jan 5, 2026

Copy link
Copy Markdown
Contributor Author

r? libs-api

@rustbot rustbot assigned the8472 and unassigned BurntSushi Jan 5, 2026
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot

rustbot commented May 20, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
tgross35
tgross35 reviewed May 27, 2026
View reviewed changes

@tgross35 tgross35 left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems reasonable to me but should get a @rust-lang/libs-api okay.

@rustbot label +I-libs-api-nominated

r? tgross35

View changes since this review

Comment thread library/core/src/bool.rs Outdated
Comment thread library/core/src/num/uint_macros.rs Outdated
Comment thread library/core/src/num/uint_macros.rs
}

/// Same value as `self | other`, but UB if any bit position is set in both inputs.
/// Disjoint, bitwise or. Computes `self | rhs`, assuming no one bits in common.

@tgross35 tgross35 May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"assuming no one bits in common" -> "assuming there are no one bits in common."

Or something like "Computes self | rhs given the precondition that no set bits of the inputs overlap."

Comment thread library/core/src/num/uint_macros.rs
/// `((a as u32) << 16) | (b as u32)`, that's fine, as the backend will
/// know those sides of the `|` are disjoint without needing help.
/// Practically, this requires that `self | rhs`, `self ^ rhs`, and `self + rhs` all
/// yield the same result, allowing for any of the three to be emitted in code gen

@tgross35 tgross35 May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: "code gen" -> either "codegen" or "code generation"

@bjoernager bjoernager May 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What is wrong with "code gen" ? It's already used in the repo: https://github.com/search?q=repo%3Arust-lang%2Frust%20%22code%20gen%22&type=code.

@tgross35 tgross35 May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most of those hits are part of "code generation" https://github.com/search?q=repo%3Arust-lang%2Frust+%2Fcode+gen%5Cb%2F&type=code. Loosely I think "gen" is considered an abbreviation rather than a word that can stand on its own, but we do consider "codegen" to be one.

Also we have codegen-units, codegen-backend etc. rather than code-gen-units and code-gen-backend.

@rustbot rustbot added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label May 27, 2026
@bjoernager
Add 'unchecked_disjoint_bitor' method to 'bool', 'i8', 'i16', 'i32', …
7d9b4e4 
…'i64', 'i128', and 'isize'; Update existing 'unchecked_disjoint_bitor' docs; Add 'must_use' to 'unchecked_disjoint_bitor';
traviscross
traviscross reviewed May 31, 2026
View reviewed changes
Comment thread library/core/src/bool.rs
if self { Ok(()) } else { Err(f()) }
}

/// Disjoint, bitwise or. Computes `self | rhs`, assuming inequality.

@traviscross traviscross May 31, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Disjoint, bitwise or. Computes `self | rhs`, assuming inequality.
/// Disjoint, bitwise or. Computes `self | rhs`, assuming at most one is `true`.

Right, or no? I'd expect (false, false) to be valid.

View changes since the review

Comment thread library/core/src/bool.rs
/// #![feature(disjoint_bitor)]
///
/// assert_eq!(
/// // SAFETY: `false` and `true` are inequal.

@traviscross traviscross May 31, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// // SAFETY: `false` and `true` are inequal.
/// // SAFETY: `false` and `true` have no bits in common.

As above.

View changes since the review

@tgross35 tgross35 Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I actually requested this move away from talking about bits with booleans, since I don't think users should need to think about the representation when thinking about values is sufficient. But looks like I gave a bad suggestion here.

So perhaps "has only one true" or "has at most one true"?

@tgross35 tgross35 Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adding a false, false test to the doc would be good too

@traviscross traviscross Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// // SAFETY: `false` and `true` are inequal.
/// // SAFETY: `false` and `true` are not both `true`.

Comment thread library/core/src/bool.rs
///
/// # Safety
///
/// This results in undefined behaviour if `self` and `rhs` are equal.

@traviscross traviscross May 31, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// This results in undefined behaviour if `self` and `rhs` are equal.
/// This results in undefined behavior if `self` and `rhs` are both `true`.

As above.

Nit: We generally use American spellings.

View changes since the review

Comment thread library/core/src/bool.rs
pub const unsafe fn unchecked_disjoint_bitor(self, rhs: Self) -> Self {
assert_unsafe_precondition!(
check_language_ub,
"bool::unchecked_disjoint_bitor cannot bitor equal values",

@traviscross traviscross May 31, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
"bool::unchecked_disjoint_bitor cannot bitor equal values",
"bool::unchecked_disjoint_bitor cannot bitor overlapping ones",

As above.

View changes since the review

@tgross35 tgross35 Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similarly, maybe "more than one true"

@traviscross traviscross Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
"bool::unchecked_disjoint_bitor cannot bitor equal values",
"bool::unchecked_disjoint_bitor cannot bitor two `true` values",

Comment thread library/core/src/bool.rs
(
lhs: bool = self,
rhs: bool = rhs,
) => lhs != rhs,

@traviscross traviscross May 31, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
) => lhs != rhs,
) => !(lhs && rhs),

Right? As above, I'd expect (false, false) to be valid.

View changes since the review

@nia-e

nia-e commented Jun 2, 2026

Copy link
Copy Markdown
Member

We discussed this in today's libs-api meeting; we're happy to add these functions. Thanks!

@tgross35

tgross35 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Just a couple of small remaining review comments

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 8, 2026
@rustbot

rustbot commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@clarfonthey clarfonthey removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@traviscross traviscross traviscross left review comments

@tgross35 tgross35 tgross35 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@tgross35 tgross35

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@bjoernager @rustbot @rust-log-analyzer @nia-e @tgross35 @traviscross @BurntSushi @the8472 @Mark-Simulacrum @clarfonthey