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

regression: trait no longer implemented #61563

Closed
Mark-Simulacrum opened this issue Jun 5, 2019 · 6 comments
Closed

regression: trait no longer implemented #61563

Mark-Simulacrum opened this issue Jun 5, 2019 · 6 comments
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Mark-Simulacrum
Copy link
Member

---- src/lib.rs - Vec<N, A>::from (line 1087) stdout ----
error[E0277]: the trait bound `sized_vec::Vec<_, _>: std::convert::From<generic_array::GenericArray<{integer}, typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UTerm, typenum::bit::B1>, typenum::bit::B1>>>` is not satisfied
 --> src/lib.rs:1095:28
  |
9 | let good_vec: Vec<U3, _> = Vec::from(array);
  |                            ^^^^^^^^^ the trait `std::convert::From<generic_array::GenericArray<{integer}, typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UTerm, typenum::bit::B1>, typenum::bit::B1>>>` is not implemented for `sized_vec::Vec<_, _>`
  |
  = help: the following implementations were found:
            <sized_vec::Vec<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UTerm, typenum::bit::B1>, typenum::bit::B0>, typenum::bit::B0>, typenum::bit::B0>, typenum::bit::B0>, A> as std::convert::From<[A; 16]>>
            <sized_vec::Vec<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UTerm, typenum::bit::B1>, typenum::bit::B0>, typenum::bit::B0>, typenum::bit::B0>, typenum::bit::B1>, A> as std::convert::From<[A; 17]>>
            <sized_vec::Vec<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UTerm, typenum::bit::B1>, typenum::bit::B0>, typenum::bit::B0>, typenum::bit::B1>, typenum::bit::B0>, A> as std::convert::From<[A; 18]>>
            <sized_vec::Vec<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UInt<typenum::uint::UTerm, typenum::bit::B1>, typenum::bit::B0>, typenum::bit::B0>, typenum::bit::B1>, typenum::bit::B1>, A> as std::convert::From<[A; 19]>>
          and 28 others
  = note: required by `std::convert::From::from`

error: aborting due to previous error

@Mark-Simulacrum Mark-Simulacrum added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jun 5, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Jun 20, 2019

This was injected between rustc 1.36.0-nightly (3991285 2019-04-25) and rustc 1.36.0-nightly (597f432 2019-04-26).

There were four PR's merged in that time period:

597f432489f12a3f33419daa039ccef11a12c4fd Auto merge of #60303 - CryZe:patch-6, r=Centril
3ee936378662bd2e74be951d6a7011a95a6bd84d Auto merge of #60240 - mati865:musl_toolchain, r=alexcrichton
e8310a77143879ef0aa5ee4a0dffb4216390432b Auto merge of #60167 - varkor:tidy-filelength, r=matthewjasper
180edc21eeca50d0d597de091c8eb712667b5dd2 Auto merge of #60296 - Centril:rollup-qh9la7k, r=Centril

And it was in fact injected specifically by the rollup #60296, which has the following PR's in it:

@pnkfelix
Copy link
Member

pnkfelix commented Jun 20, 2019

Ah... the code in question is this:

https://github.com/bodil/sized-vec/blob/617bb45990c4ae734ebabd37f9746c4399db9554/src/lib.rs#L1082

#[cfg(any(test, feature = "generic-array"))]
impl<N, A> From<generic_array::GenericArray<A, N>> for Vec<N, A>
where
    N: Unsigned + generic_array::ArrayLength<A>,
{
    /// Construct a vector of size `N` from a `GenericArray`.
    ///
    /// # Examples
    /// ```
    /// # #[macro_use] extern crate sized_vec;
    /// # use std::convert::TryFrom;
    /// # use sized_vec::Vec;
    /// # use typenum::U3;
    /// # use generic_array::GenericArray;
    /// # fn main() {
    /// let array = GenericArray::from([1, 2, 3]);
    /// let good_vec: Vec<U3, _> = Vec::from(array);
    /// assert_eq!(svec![1, 2, 3], good_vec);
    /// # }
    /// ```
...

Note in particular that this code is guarded by cfg(any(test, feature = "generic-array")).

So, before PR #59940 (which was in the rollup PR), we would not have tested that example code during rustdoc --test, because it would not have had cfg(test) set before that PR.

So presumably that code example fails to compile if one were to lift it to a higher point, outside of a doc example? I'll give that a quick whirl.

Update: (but I don't even understand how that cfg(any(test, feature = "generic-array")) can work ... how can that code sample compile at all in the scenario where we have cfg(test) set, but not have generic-array set?)

@jonas-schievink
Copy link
Contributor

That PR might be getting reverted by #61199 as well

@jonas-schievink jonas-schievink added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jun 20, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Jun 20, 2019

Well, in any case, when I lift the code in that test out of the docs, it fails to compile even on stable (and failed to compile back in 1.34. The crate fails to compile for other reasons related to unstable APIs on earlier versions.)

@pnkfelix
Copy link
Member

(I still don't really understand how the logic for the cfg on that impl is meant to work. I would think that this cfg should just say #[cfg(feature = "generic-array")]; not #[cfg(any(test, feature = "generic-array"))].)

@pnkfelix
Copy link
Member

Filed bodil/sized-vec#4 as a followup issue in the problematic crate.

Closing as not-a-bug, or at least not a regression that I think we need to act on specifically (though if we happen to "fix" it via #61199, I won't complain).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants