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.

Sign up for GitHub

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

Jump to bottom

Add slice::{from_ptr_range, from_mut_ptr_range} #89793

Merged
merged 1 commit into from
Feb 28, 2022
Merged

Add slice::{from_ptr_range, from_mut_ptr_range} #89793

merged 1 commit into from
Feb 28, 2022

Conversation

ibraheemdev
Copy link
Member

Adds slice::{from_ptr_range, from_mut_ptr_range} as counterparts to slice::{as_ptr_range, as_mut_ptr_range}.

@rust-highfive
Copy link
Collaborator

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 11, 2021
@ibraheemdev ibraheemdev mentioned this pull request Oct 11, 2021
Open
5 tasks
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@the8472 the8472 added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Oct 11, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Oct 15, 2021

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

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 31, 2021
@ibraheemdev ibraheemdev force-pushed the from_ptr_range branch 2 times, most recently from 8cc9609 to acbaae7 Compare November 13, 2021 04:03
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Dec 1, 2021

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

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 6, 2022
@ibraheemdev
Copy link
Member Author

@m-ou-se can you review this when you have the time, or should it be assigned to someone else?

m-ou-se
m-ou-se approved these changes Feb 24, 2022
View reviewed changes
Copy link
Member

@m-ou-se m-ou-se left a comment

Choose a reason for hiding this comment

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

Apologies.

This looks good to me.

There is just one issue that worries me: There have been (incomplete) plans for quite a while to replace Range by two types: One that is Copy, and one that is an iterator. Using Range in signatures like this makes that change a little bit harder than it already is. Of course, the existing (and stable) .as_ptr_range() already has this problem as it uses Range as its return type. Still, I'm worried about making a potential change to Range even harder by adding the functions in this PR.

That doesn't have to stand in the way of adding these as unstable. I have added this as an unresolved question to the tracking issue, so we can make sure to address it before stabilizing this.

If you can fix the merge conflict, this is ready to merge.

@ibraheemdev
Copy link
Member Author

Fixed @m-ou-se

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@m-ou-se
Copy link
Member

m-ou-se commented Feb 25, 2022

Can you rebase/squash the commits? Because now there's a commit named "update rust-analyzer submodule", which shouldn't be part of this PR.

@ibraheemdev
Copy link
Member Author

squashed @m-ou-se

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@m-ou-se
Copy link
Member

m-ou-se commented Feb 28, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 28, 2022

📌 Commit aac0281 has been approved by m-ou-se

@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 28, 2022
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 28, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2022
@bors
Auto merge of rust-lang#94447 - matthiaskrgr:rollup-d8rj2xv, r=matthi…
97cde9f 
…askrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#89793 (Add `slice::{from_ptr_range, from_mut_ptr_range} `)
 - rust-lang#92642 (Update search location from a relative path to absolute)
 - rust-lang#93389 (regression for issue 90847)
 - rust-lang#93413 (Fix broken link from rustdoc docs to ayu theme)
 - rust-lang#94365 (Fix MinGW target detection in raw-dylib)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 770ee32 into rust-lang:master Feb 28, 2022
@rustbot rustbot added this to the 1.61.0 milestone Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@the8472 the8472 the8472 left review comments

@m-ou-se m-ou-se m-ou-se approved these changes

Assignees

@m-ou-se m-ou-se

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.61.0
Development

Successfully merging this pull request may close these issues.
8 participants
@ibraheemdev @rust-highfive @rust-log-analyzer @bors @m-ou-se @the8472 @JohnCSimon @rustbot