-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Split core/str/mod.rs to smaller files #76325
Conversation
@rustbot modify labels: +S-waiting-on-review |
r? @KodrAus |
☔ The latest upstream changes (presumably #76964) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
067e571
to
b78c4b5
Compare
Rebased. |
☔ The latest upstream changes (presumably #77201) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
Also move FromStr trait
b78c4b5
to
dce7248
Compare
Rebased. |
@rust-lang/libs: is there someone who could review this PR? It bitrots quite quickly. @bors p=100 |
@bors r+ Also I learned something new about git today! |
📌 Commit dce7248 has been approved by |
☀️ Test successful - checks-actions, checks-azure |
This caused a 26.6% reduction on clap-rs-debug incr-patched: println. https://perf.rust-lang.org/compare.html?start=ef663a8a48ea6b98b43cbfaefd99316b36b16825&end=9bb55dc8642d811d66a7599812009cc063577e00&stat=instructions:u |
It also regresses wall-time and some max-rss tests: https://perf.rust-lang.org/compare.html?start=ef663a8a48ea6b98b43cbfaefd99316b36b16825&end=9bb55dc8642d811d66a7599812009cc063577e00&stat=wall-time I don't think we could do anything to improve it. |
@bjorn3 Is that number really due to this PR? It looks like that benchmark has been oscillating between two results, recently: |
Ah, ok. |
pub trait FromStr: Sized { | ||
/// The associated error which can be returned from parsing. | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
type Err; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this was supposed to be type Error
, is it possible to change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are 9.5 years too late to change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To elaborate: It was already type Err
before this PR. #21718 which stabilized FromStr
introduced type Err
.
Note for reviewer:
I split
core/str/mod.rs
to these modules:converts
: Contains helper functions to convert from bytes to str.error
: For error structs like Utf8Error.iter
: For iterators of many str methods.traits
: For indexing operations and build in traits on str.validations
: For functions validating utf8 --- This name is awkward, maybe utf8.rs is better.