Implement AsRef<ByteStr> for [u8] by thaliaarchi · Pull Request #139441 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation10 Commits1 Checks10 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This impl was omitted from #135073 due to inference failures. Add it separately to judge its impact.
Tracked in #139429.
r? libs-api
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
This comment has been minimized.
Some changes occurred in src/tools/clippy
cc @rust-lang/clippy
This comment has been minimized.
The failure is now from inference failures in external crates.
BurntSushi added a commit to BurntSushi/jiff that referenced this pull request
I believe the signature of the constructor was originally generic over
AsRef<[u8]>, but it got changed to be concrete. However, the
implementation continued calling as_ref() unnecessarily. This looks
harmless, but exposes us to inference failures.
Add it separately to judge its impact.
If you are looking for a crater run on this PR, we'll need a green build first before that can be kicked off. Please try updating jiff from 0.2.5 to 0.2.6 to pull in BurntSushi/jiff@f41d586.
error[E0283]: type annotations needed
--> /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/jiff-0.2.5/src/shared/posix.rs:26:42
|
26 | Parser { ianav3plus: true, ..Parser::new(bytes.as_ref()) };
| ^^^^^^^^^^^ -------------- type must be known at this point
| |
| cannot infer type of the type parameter B declared on the associated function new
|
= note: multiple impls satisfying _: AsRef<[u8]> found in the following crates: alloc, core:
- impl AsRef<[u8]> for ByteStr;
- impl AsRef<[u8]> for ByteString;
- impl AsRef<[u8]> for String;
- impl AsRef<[u8]> for str;
note: required by a bound in shared::posix::Parser::<'s>::new
--> /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/jiff-0.2.5/src/shared/posix.rs:602:24
|
602 | fn new<B: ?Sized + AsRef<[u8]>>(tz: &'s B) -> Parser<'s> {
| ^^^^^^^^^^^ required by this bound in Parser::<'s>::new
help: consider specifying the generic argument
|
26 | Parser { ianav3plus: true, ..Parser:🆕:(bytes.as_ref()) };
| +++++
help: consider removing this method call, as the receiver has type &[u8] and &[u8]: AsRef<[u8]> trivially holds
|
26 - Parser { ianav3plus: true, ..Parser::new(bytes.as_ref()) };
26 + Parser { ianav3plus: true, ..Parser::new(bytes) };
|
Status: This is awaiting some action (such as code changes or more information) from the author.
This change needs a crater run to check for possible breakage in the ecosystem.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
@thaliaarchi
Thanks for your contribution.
Form wg-triage. Any updates on this PR?
This impl was omitted from rust-lang#135073 due to inference failures. Add it separately to judge its impact.
⚠️ Warning ⚠️
- There are issue links (such as
#123) in the commit messages of the following commits.
Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.
The job aarch64-gnu-llvm-19-1 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Compiling gix-index v0.41.0
error[E0282]: type annotations needed
--> /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/sec1-0.7.3/src/point.rs:532:14
|
532 | if y.as_ref().last().expect("empty y-coordinate") & 1 == 1 {
| ^^^^^^ ---- type must be known at this point
|
help: try using a fully qualified path to specify the expected types
|
532 - if y.as_ref().last().expect("empty y-coordinate") & 1 == 1 {
532 + if <[u8] as AsRef<T>>::as_ref(y).last().expect("empty y-coordinate") & 1 == 1 {
|
For more information about this error, try `rustc --explain E0282`.
[RUSTC-TIMING] sec1 test:false 0.347
error: could not compile `sec1` (lib) due to 1 previous error
I rebased to incorporate the jiff version bump on master. The failure is now with the sec1 crate.
error[E0282]: type annotations needed
--> /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/sec1-0.7.3/src/point.rs:532:14
|
532 | if y.as_ref().last().expect("empty y-coordinate") & 1 == 1 {
| ^^^^^^ ---- type must be known at this point
|
help: try using a fully qualified path to specify the expected types
|
532 - if y.as_ref().last().expect("empty y-coordinate") & 1 == 1 {
532 + if <[u8] as AsRef<T>>::as_ref(y).last().expect("empty y-coordinate") & 1 == 1 {
|
Is the path forward for this PR then to upstream fixes in each crate that r-l/r depends on, then run crater, then to consider it?
Is the path forward for this PR then to upstream fixes in each crate that r-l/r depends on, then run crater, then to consider it?
Upstreaming fixes in each crate that rust-lang/rust depends on is a prerequisite for running crater, and a reassuring crater result is a prerequisite for the library API team approving this impl. You are free to pursue that as long as you want. Based on my experience there is basically 0 possibility that crater will demonstrate that it is acceptable for us to approve this impl.
Labels
This change needs a crater run to check for possible breakage in the ecosystem.
Status: This is awaiting some action (such as code changes or more information) from the author.
Relevant to the Clippy team.
Relevant to the library team, which will review and decide on the PR/issue.