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

impl<'a> From<&'a BString> for Cow<'a, BStr> #187

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

wbenny
Copy link
Contributor

@wbenny wbenny commented Jul 25, 2024

This commit simplifies code in situations like:

let mut v = Vec::<Cow<'a, BStr>>::new();
let s = BString::new(...);

// Before this commit, we would have to do:
// v.push(s.as_bstr().into());
v.push(s.into());

This commit simplifies code in situations like:

```
let mut v = Vec::<Cow<'a, BStr>>::new();
let s = BString::new(...);

// Before this commit, we would have to do:
// v.push(s.as_bstr().into());
v.push(s.into());
```
Copy link
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

Makes sense to me, and lines up with the corresponding impl in std.

@BurntSushi BurntSushi merged commit 06b1f14 into BurntSushi:master Jul 25, 2024
8 checks passed
@BurntSushi
Copy link
Owner

This PR is on crates.io in bstr 1.9.2.

@wbenny
Copy link
Contributor Author

wbenny commented Jul 25, 2024

Makes sense to me, and lines up with the corresponding impl in std.

Yes, exactly, forgot to mention that.

Huge thank you for releasing the crate so fast!

Cheers

@epage
Copy link

epage commented Jul 25, 2024

I upgraded cargo to bstr 1.9.2 and it broke the build of gix

cargo on  deps is 📦 v0.83.0
❯ cargo check
    Checking gix-date v0.8.6
    Checking gix-credentials v0.24.2
error[E0283]: type annotations needed
  --> /home/epage/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gix-credentials-0.24.2/src/program/mod.rs:83:63
   |
83 |                 gix_command::prepare(gix_path::from_bstr(args.as_ref()).into_owned())
   |                                                               ^^^^^^
   |
   = note: multiple `impl`s satisfying `BString: AsRef<_>` found in the `bstr` crate:
           - impl AsRef<BStr> for BString;
           - impl AsRef<[u8]> for BString;
help: try using a fully qualified path to specify the expected types
   |
83 |                 gix_command::prepare(gix_path::from_bstr(<BString as AsRef<T>>::as_ref(&args)).into_owned())
   |                                                          +++++++++++++++++++++++++++++++    ~

error[E0283]: type annotations needed
   --> /home/epage/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gix-credentials-0.24.2/src/program/mod.rs:83:3
8
    |
83  |                 gix_command::prepare(gix_path::from_bstr(args.as_ref()).into_owned())
    |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type for reference `&_`
    |
    = note: multiple `impl`s satisfying `Cow<'_, BStr>: From<&_>` found in the `bstr` crate:
            - impl<'a> From<&'a BStr> for Cow<'a, BStr>;
            - impl<'a> From<&'a BString> for Cow<'a, BStr>;
    = note: required for `&_` to implement `Into<Cow<'_, BStr>>`
note: required by a bound in `from_bstr`
   --> /home/epage/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gix-path-0.10.7/src/convert.rs:135:34
    |
135 | pub fn from_bstr<'a>(input: impl Into<Cow<'a, BStr>>) -> Cow<'a, Path> {
    |                                  ^^^^^^^^^^^^^^^^^^^ required by this bound in `from_bstr`

    Checking gix-actor v0.31.2
For more information about this error, try `rustc --explain E0283`.
error: could not compile `gix-credentials` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...

Would this be considered

  • A major incompatibility (breaking change)
  • A minor incompatibility
  • An issue within gix

@BurntSushi
Copy link
Owner

Uuuuuuuuuuggggggg. That's "incorrect" as_ref() usage.

I'll yank bstr 1.9.2 for now, but I don't think this can be a permanent thing. Otherwise I can basically never add any new trait impls. People should only be using as_ref() when the target type is constrained.

@BurntSushi
Copy link
Owner

OK, it has been yanked. But yes, I generally consider this an issue in gix.

@BurntSushi
Copy link
Owner

I filed an issue with gix: Byron/gitoxide#1466

@BurntSushi
Copy link
Owner

Sorry @wbenny. Hopefully we can get this trait impl published soon, but given this was caught early and yanking in this specific case doesn't really have too much downside, I want to avoid creating an emergency for gitoxide. The quickest way is probably landing a PR fixing the specific error @epage ran into and getting them to do a release. Then I can publish a new release of bstr. That way, there will at least be a release of gitoxide that is compatible with bstr.

@BurntSushi
Copy link
Owner

OK, let's try this again. This PR is on crates.io in bstr 1.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants