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

Restrict DerefPure for Cow<T> impl to T = impl Clone, [impl Clone], str. #137105

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

zachs18
Copy link
Contributor

@zachs18 zachs18 commented Feb 16, 2025

Fixes #136046

feature(deref_patterns) tracking issue: #87121

Cow<'_, T> should only implement DerefPure if its Deref impl is pure, which requires <T::Owned as Borrow<T>>::borrow to be pure. This PR restricts impl DerefPure for Cow<'_, T> to T: Sized + Clone, T = [U: Clone], and T = str (for all of whom <T::Owned as Borrow<T>>::borrow is implemented in the stdlib and is pure).

cc @Nadrieril


An alternate approach would be to introduce a new unsafe trait BorrowPure<T> analogous to DerefPure that could be implemented for T: Sized, &T, &mut T, String, Vec, Box, PathBuf, OsString, etc. master...zachs18:borrow-pure-trait

@rustbot
Copy link
Collaborator

rustbot commented Feb 16, 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 Feb 16, 2025
@rust-log-analyzer

This comment has been minimized.

@zachs18 zachs18 force-pushed the cow-derefpure-restrict branch from 594cc22 to 0f220ef Compare February 16, 2025 03:02
@Nadrieril
Copy link
Member

Nadrieril commented Feb 16, 2025

Thanks!

Despite this looking like a t-libs change, DerefPure is for now an implementation detail of an unstable feature so I'm taking the initiative of merging this myself (as a t-compiler member). Is that ok? If this trait were to be stabilized or have more complex interactions with other features then t-libs would ofc be in charge.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 16, 2025

📌 Commit 0f220ef has been approved by Nadrieril

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 Feb 16, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Feb 16, 2025
…Nadrieril

Restrict DerefPure for Cow<T> impl to T = impl Clone, [impl Clone], str.

Fixes rust-lang#136046

`feature(deref_patterns)` tracking issue: rust-lang#87121

`Cow<'_, T>` should only implement `DerefPure` if its `Deref` impl is pure, which requires `<T::Owned as Borrow<T>>::borrow`  to be pure. This PR restricts `impl DerefPure for Cow<'_, T>` to `T: Sized + Clone`, `T = [U: Clone]`, and `T = str` (for all of whom `<T::Owned as Borrow<T>>::borrow` is implemented in the stdlib and is pure).

cc `@Nadrieril`

------

An alternate approach would be to introduce a new `unsafe trait BorrowPure<T>` analogous to `DerefPure`  that could be implemented for `T: Sized`, `&T`, `&mut T`, `String`, `Vec`, `Box`, `PathBuf`, `OsString`, etc. rust-lang/rust@master...zachs18:borrow-pure-trait
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 17, 2025
…kingjubilee

Rollup of 12 pull requests

Successful merges:

 - rust-lang#136671 (Overhaul `rustc_middle::limits`)
 - rust-lang#136817 (Pattern Migration 2024: clean up and comment)
 - rust-lang#136844 (Use `const_error!` when possible)
 - rust-lang#136953 (rustc_target: import TargetMetadata)
 - rust-lang#137095 (Replace some u64 hashes with Hash64)
 - rust-lang#137100 (HIR analysis: Remove unnecessary abstraction over list of clauses)
 - rust-lang#137105 (Restrict DerefPure for Cow<T> impl to T = impl Clone, [impl Clone], str.)
 - rust-lang#137120 (Enable `relative-path-include-bytes-132203` rustdoc-ui test on Windows)
 - rust-lang#137125 (Re-add missing empty lines in the releases notes)
 - rust-lang#137140 (Fix const items not being allowed to be called `r#move` or `r#static`)
 - rust-lang#137145 (use add-core-stubs / minicore for a few more tests)
 - rust-lang#137149 (Remove SSE ABI from i586-pc-windows-msvc)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 17, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#137095 (Replace some u64 hashes with Hash64)
 - rust-lang#137100 (HIR analysis: Remove unnecessary abstraction over list of clauses)
 - rust-lang#137105 (Restrict DerefPure for Cow<T> impl to T = impl Clone, [impl Clone], str.)
 - rust-lang#137120 (Enable `relative-path-include-bytes-132203` rustdoc-ui test on Windows)
 - rust-lang#137125 (Re-add missing empty lines in the releases notes)
 - rust-lang#137145 (use add-core-stubs / minicore for a few more tests)
 - rust-lang#137149 (Remove SSE ABI from i586-pc-windows-msvc)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fae72a0 into rust-lang:master Feb 17, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Feb 17, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 17, 2025
Rollup merge of rust-lang#137105 - zachs18:cow-derefpure-restrict, r=Nadrieril

Restrict DerefPure for Cow<T> impl to T = impl Clone, [impl Clone], str.

Fixes rust-lang#136046

`feature(deref_patterns)` tracking issue: rust-lang#87121

`Cow<'_, T>` should only implement `DerefPure` if its `Deref` impl is pure, which requires `<T::Owned as Borrow<T>>::borrow`  to be pure. This PR restricts `impl DerefPure for Cow<'_, T>` to `T: Sized + Clone`, `T = [U: Clone]`, and `T = str` (for all of whom `<T::Owned as Borrow<T>>::borrow` is implemented in the stdlib and is pure).

cc ``@Nadrieril``

------

An alternate approach would be to introduce a new `unsafe trait BorrowPure<T>` analogous to `DerefPure`  that could be implemented for `T: Sized`, `&T`, `&mut T`, `String`, `Vec`, `Box`, `PathBuf`, `OsString`, etc. rust-lang/rust@master...zachs18:borrow-pure-trait
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.

DerefPure impl for Cow may be too general
6 participants