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

Untested code in docs #4

Closed
pnkfelix opened this issue Jun 20, 2019 · 1 comment
Closed

Untested code in docs #4

pnkfelix opened this issue Jun 20, 2019 · 1 comment

Comments

@pnkfelix
Copy link

Spawned off of rust-lang/rust#61563

If you look at:

sized-vec/src/lib.rs

Lines 1082 to 1101 in 617bb45

#[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);
/// # }
/// ```

Under the current beta version of rust 1.36, we get an error from cargo test on this crate due to line 1098.

But as far as I can tell, the reason we are seeing this error now is not because of a regression in the compiler's behavior, but rather because this documentation example is dead code by default. The cfg flag at the front, under 1.35 and earlier, cause rustdoc not to include it. Right now rustdoc might include it in 1.36 ("might" because of rust-lang/rust#61199 ), but the real point here is that this code example is not being tested by default, and that might represent a bug somewhere here overall.

Maybe the impl there should be guarded by just cfg(feature = "generic-array")? Why is it cfg(any(test, feature = "generic-array")) ?

@bodil
Copy link
Owner

bodil commented Jun 20, 2019

The cfg checks for test because I don't want to have to use the --all-features flag on cargo test, and generic-array is a dev-dependency as well as an optional dependency, which I'd have expected to be available when testing. You're right, though, that test seems to have been ignored up until now, and, more curiously, on current nightly cargo test fails as you've noted, while cargo test --all-features compiles and passes the test, so it would seem rustdoc doesn't include my listed dev-dependencies.

So I think I'm just going to make sure CI uses cargo test --all-featuresand remove the test guard from the cfg.

@bodil bodil closed this as completed in 4596565 Jun 20, 2019
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

No branches or pull requests

2 participants