Skip to content

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

Merged
BurntSushi merged 1 commit intoBurntSushi:masterfrom
wbenny:master
Jul 25, 2024
Merged

impl<'a> From<&'a BString> for Cow<'a, BStr>#187
BurntSushi merged 1 commit intoBurntSushi:masterfrom
wbenny:master

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
@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: GitoxideLabs/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