Skip to content

Implement AsRef<ByteStr> for [u8] #139441

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thaliaarchi
Copy link
Contributor

@thaliaarchi thaliaarchi commented Apr 6, 2025

This impl was omitted from #135073 due to inference failures. Add it separately to judge its impact.

Tracked in #139429.

r? libs-api

cc @joshtriplett

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 6, 2025
@rust-log-analyzer

This comment has been minimized.

@thaliaarchi thaliaarchi force-pushed the asref/slice-to-byte_str branch from 0e7022a to 3317391 Compare April 6, 2025 05:58
@rustbot
Copy link
Collaborator

rustbot commented Apr 6, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

@thaliaarchi
Copy link
Contributor Author

The failure is now from inference failures in external crates.

BurntSushi added a commit to BurntSushi/jiff that referenced this pull request Apr 6, 2025
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.

Ref rust-lang/rust#139441
@dtolnay
Copy link
Member

dtolnay commented Apr 13, 2025

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 `impl`s 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::new::<B>(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) };
    |

@dtolnay dtolnay added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. needs-crater 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 Apr 13, 2025
@alex-semenyuk
Copy link
Member

@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.
@thaliaarchi thaliaarchi force-pushed the asref/slice-to-byte_str branch from 3317391 to ff47b93 Compare August 13, 2025 00:36
@rustbot rustbot added the T-clippy Relevant to the Clippy team. label Aug 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 13, 2025

⚠️ 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.

@thaliaarchi thaliaarchi force-pushed the asref/slice-to-byte_str branch 2 times, most recently from d7b8c41 to da39b9f Compare August 13, 2025 00:40
@rust-log-analyzer
Copy link
Collaborator

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

@thaliaarchi
Copy link
Contributor Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-crater This change needs a crater run to check for possible breakage in the ecosystem. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-clippy Relevant to the Clippy team. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants