Skip to content
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.

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

Turn Cow::is_borrowed,is_owned into associated functions. #138217

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

theemathas
Copy link
Contributor

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

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`.
@rustbot
Copy link
Collaborator

rustbot commented Mar 8, 2025

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 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
Copy link
Collaborator

rustbot commented Mar 8, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@theemathas
Copy link
Contributor Author

r? libs-api

@rustbot 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 rustbot assigned joshtriplett and unassigned Noratrieb Mar 8, 2025
@joshtriplett
Copy link
Member

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
Copy link
Contributor

bors commented Mar 8, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. 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.

5 participants