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

SplitInclusive is public API #83372

Merged
merged 1 commit into from
Mar 22, 2021
Merged

SplitInclusive is public API #83372

merged 1 commit into from
Mar 22, 2021

Conversation

eggyal
Copy link
Contributor

@eggyal eggyal commented Mar 22, 2021

No description provided.

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(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 Mar 22, 2021
@eggyal
Copy link
Contributor Author

eggyal commented Mar 22, 2021

@rustbot modify labels to +beta-nominated +A-visibility +T-libs

@rustbot
Copy link
Collaborator

rustbot commented Mar 22, 2021

Error: Label beta-nominated can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@eggyal
Copy link
Contributor Author

eggyal commented Mar 22, 2021

@rustbot modify labels to +A-visibility +T-libs
cc @rust-lang/release as I think this ought to be beta-nominated too?

@rustbot rustbot added A-visibility Area: Visibility / privacy T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 22, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 22, 2021

@eggyal 1.51 is being promoted to stable today, monday: https://forge.rust-lang.org/release/process.html.

cc @rust-lang/release - I don't know if this is worth dealing with at such a late point.

@jonas-schievink jonas-schievink added beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Mar 22, 2021
@Mark-Simulacrum Mark-Simulacrum added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Mar 22, 2021
@Mark-Simulacrum
Copy link
Member

Trying to figure out context here - looks like this was just overlooked when stabilizing or so, and SplitInclusive is only exposed in core and not std.

I'm going to unilaterally beta-accept (we've not released a stable with this omission yet so going to drop the stable-nomination. The beta-backport will apply to both 1.51 and 1.52, though.

cc @rust-lang/libs so y'all are aware of this

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 22, 2021

📌 Commit fe95735 has been approved by Mark-Simulacrum

@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 Mar 22, 2021
@Mark-Simulacrum Mark-Simulacrum removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Mar 22, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 22, 2021 •
edited
Loading

Trying to figure out context here - looks like this was just overlooked when stabilizing or so, and SplitInclusive is only exposed in core and not std.

Yes, it was missed in #77858. cc @ijackson

@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 22, 2021
Merged
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Mar 22, 2021
Merged
@ijackson
Copy link
Contributor

Sorry, I did indeed not spot the missing pub so I didn't make that public in std in #77858.

I'm kind of surprised that you can have a stability attribute on a non-public item at all, without generating some kind of lint.

@eggyal Thanks for fixing this.

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2021
@bors
Auto merge of rust-lang#83376 - Dylan-DPC:rollup-s2fsjwj, r=Dylan-DPC
2287a88 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#82374 (Add license metadata for std dependencies)
 - rust-lang#82683 (Document panicking cases for integer division and remainder)
 - rust-lang#83272 (Clarify non-exact length in the Iterator::take documentation)
 - rust-lang#83338 (Fix test for rust-lang#82270)
 - rust-lang#83351 (post-drop-elab check-const: explain why we still check qualifs)
 - rust-lang#83367 (Improve error message for unassigned query provider)
 - rust-lang#83372 (SplitInclusive is public API)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@jyn514 jyn514 mentioned this pull request Mar 22, 2021
Open
@eggyal
Copy link
Contributor Author

eggyal commented Mar 22, 2021

No problem, @ijackson. Also as Mark mentioned above, SplitInclusive is only exposed in core not std—is that intentional?

@bors bors merged commit ce06787 into rust-lang:master Mar 22, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 22, 2021
@eggyal eggyal deleted the split-inclusive branch March 22, 2021 18:54
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2021
@bors
Auto merge of rust-lang#83377 - Mark-Simulacrum:stable-next, r=Mark-S…
9bb6d54 
…imulacrum

[stable] 1.51.0 release

Also includes backports of the release notes, as well as:

*  SplitInclusive is public API rust-lang#83372
*  std: Fix a bug on the wasm32-wasi target opening files rust-lang#82804
*  Fix io::copy specialization using copy_file_range when writer was opened with O_APPEND rust-lang#82417

r? `@Mark-Simulacrum`
This was referenced Mar 22, 2021
Closed
Closed
@ijackson
Copy link
Contributor

No problem, @ijackson. Also as Mark mentioned above, SplitInclusive is only exposed in core not std—is that intentional?

Err, wait, that is different to the missing pub? No, that is not intentional. I am very confused, sorry!

@ijackson
Copy link
Contributor

I properly UTSL with actual coffee today and, err, ISWYM. MR in a moment.

@ijackson ijackson mentioned this pull request Mar 23, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2021
@bors
Auto merge of rust-lang#83408 - ijackson:expose-splitinclusive, r=dto…
680d9fc 
…lnay

Expose str::SplitInclusive in alloc and therefore in std

This seems to have been omitted from the beginning when this feature was first introduced in 86bf962.  Most users won't need to name this type which is probably why this wasn't noticed in the meantime.

See rust-lang#83372 for a different but related bug.

### Notes for reviewers

I think I have got this right but TBH I am not very familiar with the relationship between core and std and so on.  <strike>I also haven't don't any kind of test (not even a build) yet.  I will do a local docs build to see that the type now appears in the std docs.</strike>  I did a local docs build and it has made this type appear as `std::str::SplitInclusive` as expected

The linkification of the return value from `str::split_inclusive` teleports me to the online url for `core::str::SplitInclusive`.  I think this may be a rustdoc anomaly (similar to rust-lang#79630 maybe) but I am not sure.  Perhaps it means I haven't done the `std` -> `core` referrence correctly.

I made this insta-stable since it seems like simply a bug.  Please LMK if that is not right.  *(edited to add:)* In particular, IDK how this ought to relate to the (?)current release process.
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 26, 2021
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.53.0, 1.52.0 Mar 26, 2021
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Mar 26, 2021
Merged
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.52.0, 1.51.0 Mar 26, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 26, 2021
@bors
Auto merge of rust-lang#83516 - Mark-Simulacrum:beta-next, r=Mark-Sim…
4f27db6 
…ulacrum

[beta] backports

Initial round of beta backports, with 1 PR:

* SplitInclusive is public API rust-lang#83372

This also includes a bump to the released stable compiler.

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dtolnay dtolnay

Labels
A-visibility Area: Visibility / privacy beta-accepted Accepted for backporting to the compiler in the beta channel. 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.51.0
Development

Successfully merging this pull request may close these issues.
9 participants
@eggyal @rust-highfive @rustbot @jyn514 @Mark-Simulacrum @bors @ijackson @jonas-schievink @dtolnay