Turn `Cow::is_borrowed,is_owned` into associated functions. by theemathas · Pull Request #138217 · rust-lang/rust

theemathas · 2025-03-08T09:56:06Z

This is done because Cow implements Deref. Therefore, to avoid conflicts with an inner type having a method of the same name, we use an associated method, like Box::into_raw.

Tracking issue: #65143

rustbot · 2025-03-08T09:56:10Z

r? @Noratrieb

rustbot has assigned @Noratrieb.
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 · 2025-03-08T09:56:12Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

theemathas · 2025-03-08T09:58:22Z

r? libs-api

joshtriplett · 2025-03-08T11:08:56Z

So, +1 for documenting that users might want to call the functions this way if there's a conflict and they want to be explicit, but -1 for forcing the functions to be called this way. Yes, I understand that Cow implements Deref and these would shadow inner functions. However, we already have some methods on Cow: into_owned and to_mut. This doesn't in practice seem to pose a problem.

Did a crater run turn up some issue with these two new methods?

I also think these two new methods aren't especially useful if they can't be written in postfix form; at that point, you're not far off from just using matches! or let chains or waiting for is. So if there's some issue that stops us from making these methods, I think we should remove them entirely.

bors · 2025-03-08T16:35:36Z

☔ The latest upstream changes (presumably #138208) made this pull request unmergeable. Please resolve the merge conflicts.

apiraino · 2025-05-22T10:28:22Z

removing t-compiler as these changes seems completely in purview of libs team (feel free to request a review from t-compiler if this is not the case!)

@rustbot label -t-compiler

joshtriplett · 2025-07-01T03:12:48Z

Nominating this for discussion.

I think the two reasonable options here are:

Assuming these methods don't provoke any issues in a crater run, close this, and stabilize the methods.
If having these as methods proves to be an issue, then we should remove the methods entirely, as they don't provide any substantive value if they're not postfix.

joshtriplett · 2025-07-01T15:27:56Z

We discussed this in today's @rust-lang/libs-api meeting. Summary:

Some people had blocking objections to keeping these as methods, even if there's no breakage in a crater run, because of Cow having Deref to arbitrary user types. (Thus, there's no point in doing the crater run.)
Given the above, some would prefer to rip these out entirely, while others saw value in keeping them as associated functions (e.g. because they'd prefer not to write matches!). We didn't reach a consensus on that.
We generally agreed that if we can ship the is syntax in a timely fashion, we'd prefer that over methods like this.

So, path forward:

Merge this PR (once the conflict is fixed).
Add an unresolved question on the tracking issue, to decide before stabilization whether we want to ship these or defer to matches! and the future possibility of is.

This is done because `Cow` implements `Deref`. Therefore, to avoid conflicts with an inner type having a method of the same name, we use an associated method, like `Box::into_raw`.

theemathas · 2025-07-07T04:29:57Z

Rebased and ready for review.

theemathas · 2025-10-29T13:23:31Z

r? libs-api

dtolnay

Thank you!

View changes since this review

dtolnay · 2025-10-30T03:00:57Z

@bors r+

bors · 2025-10-30T03:01:00Z

📌 Commit b335056 has been approved by dtolnay

It is now in the queue for this repository.

…ssociated, r=dtolnay Turn `Cow::is_borrowed,is_owned` into associated functions. This is done because `Cow` implements `Deref`. Therefore, to avoid conflicts with an inner type having a method of the same name, we use an associated method, like `Box::into_raw`. Tracking issue: rust-lang#65143

Rollup of 9 pull requests Successful merges: - #138217 (Turn `Cow::is_borrowed,is_owned` into associated functions.) - #147858 (Micro-optimization attempt in coroutine layout computation) - #147923 (Simplify rustc_public context handling) - #147935 (Add LLVM realtime sanitizer) - #148115 (rustdoc: Rename unstable option `--nocapture` to `--no-capture` in accordance with `libtest`) - #148137 (Couple of changes for Redox OS) - #148176 ([rustdoc] Include attribute and derive macros when filtering on "macros") - #148193 (Remove `QPath::LangItem`) - #148253 (Handle default features and -Ctarget-features in the dummy backend) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 8 pull requests Successful merges: - #138217 (Turn `Cow::is_borrowed,is_owned` into associated functions.) - #147858 (Micro-optimization attempt in coroutine layout computation) - #147923 (Simplify rustc_public context handling) - #148115 (rustdoc: Rename unstable option `--nocapture` to `--no-capture` in accordance with `libtest`) - #148137 (Couple of changes for Redox OS) - #148176 ([rustdoc] Include attribute and derive macros when filtering on "macros") - #148253 (Handle default features and -Ctarget-features in the dummy backend) - #148272 (Align VEX V5 boot routine to 4 bytes) r? `@ghost` `@rustbot` modify labels: rollup

bors · 2025-10-30T10:03:14Z

⌛ Testing commit b335056 with merge 72fe2ff...

Rollup merge of #138217 - theemathas:cow_is_owned_borrowed_associated, r=dtolnay Turn `Cow::is_borrowed,is_owned` into associated functions. This is done because `Cow` implements `Deref`. Therefore, to avoid conflicts with an inner type having a method of the same name, we use an associated method, like `Box::into_raw`. Tracking issue: #65143

Rollup of 8 pull requests Successful merges: - rust-lang/rust#138217 (Turn `Cow::is_borrowed,is_owned` into associated functions.) - rust-lang/rust#147858 (Micro-optimization attempt in coroutine layout computation) - rust-lang/rust#147923 (Simplify rustc_public context handling) - rust-lang/rust#148115 (rustdoc: Rename unstable option `--nocapture` to `--no-capture` in accordance with `libtest`) - rust-lang/rust#148137 (Couple of changes for Redox OS) - rust-lang/rust#148176 ([rustdoc] Include attribute and derive macros when filtering on "macros") - rust-lang/rust#148253 (Handle default features and -Ctarget-features in the dummy backend) - rust-lang/rust#148272 (Align VEX V5 boot routine to 4 bytes) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 8 pull requests Successful merges: - rust-lang#138217 (Turn `Cow::is_borrowed,is_owned` into associated functions.) - rust-lang#147858 (Micro-optimization attempt in coroutine layout computation) - rust-lang#147923 (Simplify rustc_public context handling) - rust-lang#148115 (rustdoc: Rename unstable option `--nocapture` to `--no-capture` in accordance with `libtest`) - rust-lang#148137 (Couple of changes for Redox OS) - rust-lang#148176 ([rustdoc] Include attribute and derive macros when filtering on "macros") - rust-lang#148253 (Handle default features and -Ctarget-features in the dummy backend) - rust-lang#148272 (Align VEX V5 boot routine to 4 bytes) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 8 pull requests Successful merges: - rust-lang/rust#138217 (Turn `Cow::is_borrowed,is_owned` into associated functions.) - rust-lang/rust#147858 (Micro-optimization attempt in coroutine layout computation) - rust-lang/rust#147923 (Simplify rustc_public context handling) - rust-lang/rust#148115 (rustdoc: Rename unstable option `--nocapture` to `--no-capture` in accordance with `libtest`) - rust-lang/rust#148137 (Couple of changes for Redox OS) - rust-lang/rust#148176 ([rustdoc] Include attribute and derive macros when filtering on "macros") - rust-lang/rust#148253 (Handle default features and -Ctarget-features in the dummy backend) - rust-lang/rust#148272 (Align VEX V5 boot routine to 4 bytes) r? `@ghost` `@rustbot` modify labels: rollup

…ssociated, r=dtolnay Turn `Cow::is_borrowed,is_owned` into associated functions. This is done because `Cow` implements `Deref`. Therefore, to avoid conflicts with an inner type having a method of the same name, we use an associated method, like `Box::into_raw`. Tracking issue: rust-lang#65143

rustbot assigned Noratrieb Mar 8, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 8, 2025

rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 8, 2025

rustbot assigned joshtriplett and unassigned Noratrieb Mar 8, 2025

hkBst mentioned this pull request Mar 9, 2025

Tracking issue for Cow::is_borrowed and Cow::is_owned #65143

Open

rustbot removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 22, 2025

joshtriplett added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jul 1, 2025

Turn Cow::is_borrowed,is_owned into associated functions.

b335056

This is done because `Cow` implements `Deref`. Therefore, to avoid conflicts with an inner type having a method of the same name, we use an associated method, like `Box::into_raw`.

theemathas force-pushed the cow_is_owned_borrowed_associated branch from 49e853b to b335056 Compare July 7, 2025 04:27

theemathas mentioned this pull request Jul 7, 2025

./x suggest --run produces incorrect test failure #143569

Closed

Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jul 8, 2025

rustbot assigned dtolnay and unassigned joshtriplett Oct 29, 2025

dtolnay approved these changes Oct 30, 2025

View reviewed changes

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 30, 2025

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 30, 2025

Zalathar mentioned this pull request Oct 30, 2025

Rollup of 9 pull requests #148276

Closed

jhpratt mentioned this pull request Oct 30, 2025

Rollup of 8 pull requests #148280

Merged

bors merged commit f1a0dfd into rust-lang:master Oct 30, 2025
11 checks passed

rustbot added this to the 1.93.0 milestone Oct 30, 2025

github-actions bot mentioned this pull request Nov 4, 2025

Toolchain upgrade to nightly-2026-01-30 flux-rs/flux#1345

Draft

theemathas deleted the cow_is_owned_borrowed_associated branch November 14, 2025 13:46

Uh oh!

Conversation

theemathas commented Mar 8, 2025

Uh oh!

rustbot commented Mar 8, 2025

Uh oh!

rustbot commented Mar 8, 2025

Uh oh!

theemathas commented Mar 8, 2025

Uh oh!

joshtriplett commented Mar 8, 2025

Uh oh!

bors commented Mar 8, 2025

Uh oh!

apiraino commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshtriplett commented Jul 1, 2025

Uh oh!

joshtriplett commented Jul 1, 2025

Uh oh!

theemathas commented Jul 7, 2025

Uh oh!

theemathas commented Oct 29, 2025

Uh oh!

dtolnay left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dtolnay commented Oct 30, 2025

Uh oh!

bors commented Oct 30, 2025

Uh oh!

Uh oh!

bors commented Oct 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

apiraino commented May 22, 2025 •

edited

Loading

dtolnay left a comment •

edited by rustbot

Loading