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

Implement RFC 2500 Needle API (Part 1) #76901

Closed
wants to merge 5 commits into from

Conversation

crlf0710
Copy link
Member

@crlf0710 crlf0710 commented Sep 19, 2020

This is a rebase of #59591 by @kennytm . As that PR is relatively large, i'm splitting it into a series smaller PRs.

This is the first PR in the series, which defines the core functions and traits. I've done the necessary changes like accounting for min_specialization, unsafe_op_in_unsafe_fn and i also documented all the unsafe blocks.

Since most of the code is not written by myself, the code should definitely needs more eyes on it.

Link to RFC 2500.

r? @Amanieu cc @withoutboats

@crlf0710 crlf0710 added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-patterns Relating to patterns and pattern matching labels Sep 19, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 19, 2020
@Amanieu
Copy link
Member

Amanieu commented Sep 23, 2020

I don't have the time to do much reviewing at the moment. Maybe @BurntSushi can take a look at this since this is related to regex?

///
/// # Examples
///
/// Implement a searcher and consumer which matches `b"Aaaa"` from a byte string.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should explain what is a consumer? I don't know if consumer can be an easy term to understand since there are many other ways that a consumer can be used, I wonder if there is a better term that can be used here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think i'll just link searcher and consumer to the corresponding trait...

@camelid camelid 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 16, 2020
@camelid
Copy link
Member

camelid commented Oct 16, 2020

r? @BurntSushi

@rust-highfive rust-highfive assigned BurntSushi and unassigned Amanieu Oct 16, 2020
@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2020
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Nov 20, 2020
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Dec 11, 2020
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jan 15, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Feb 17, 2021

r? @KodrAus

@rust-highfive rust-highfive assigned KodrAus and unassigned BurntSushi Feb 17, 2021
@KodrAus
Copy link
Contributor

KodrAus commented Feb 21, 2021

I'll spend some time looking through this. I think there's a chance we'll want to try adopt a much simpler public interface for the API based on https://gist.github.com/rust-play/6f4932a58832875b81e37306cf6f16cd

@crlf0710 crlf0710 added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jun 5, 2021
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jun 25, 2021
@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jul 9, 2021
@joshtriplett
Copy link
Member

We discussed this in several recent @rust-lang/libs-api meetings, including last week and this week. This has been on our agenda for a long time.

We feel that this was originally proposed with the intention of providing integration between the standard library and (for instance) regex crates. It's become clearer, since then, that people seem fine using such functionality from regex crates rather than from the standard library.

Thus, we don't feel that we should add and stabilize this substantial additional API surface area.

We would instead propose sealing this trait, making it possible to name (if you want to accept instances of it in your own method that calls a standard library method) but not to implement for any type outside the standard library.

@rfcbot close

@rfcbot
Copy link

rfcbot commented Jul 14, 2021

Team member @joshtriplett has proposed to close this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels Jul 14, 2021
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jul 14, 2021
@rfcbot
Copy link

rfcbot commented Jul 14, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@joshtriplett joshtriplett removed the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jul 21, 2021
@jhwgh1968
Copy link

jhwgh1968 commented Jul 23, 2021

Since my attempt to ping the team over there didn't work, I'll just briefly mention that I hope this reverting of stabilization does not affect #49802 as well. I could really use several parts of that.

@inquisitivecrystal
Copy link
Contributor

inquisitivecrystal commented Jul 24, 2021

I'm sad to see this go. It's always seemed like opening this up would be really nice and make everything easier to work with. On the other hand, I'm not the one who would need to maintain it.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jul 24, 2021
@rfcbot
Copy link

rfcbot commented Jul 24, 2021

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Jul 24, 2021
@Kixunil
Copy link
Contributor

Kixunil commented Jul 25, 2021

Sorry for late question, wouldn't sealing the trait prevent regex crates from existing? Am I missing something?

@inquisitivecrystal
Copy link
Contributor

Sorry for late question, wouldn't sealing the trait prevent regex crates from existing? Am I missing something?

@Kixunil Nah, nothing like that. All it will do is prevent regex crates from being usable by standard library APIs. For instance, you'd need to type regex.split(string) instead of string.split(regex). This is actually the status quo. It had been planned to change it, but now those plans are being canceled.

@crlf0710
Copy link
Member Author

Closing according to fcp decision above.
The next steps will be merging rust-lang/rfcs#3154

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-patterns Relating to patterns and pattern matching disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. finished-final-comment-period The final comment period is finished for this 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.