Expand Clippy coverage to docs features and resolve more lints

[adamreichold]

No description provided.

[adamreichold]

This is not yet triggered on beta, but on nightly this hits the deny by default lint uninit_vec. The change does not really fix anything as the problem is inherent in the signature of the deprecated method but rather works around this by deduplicating the implementation so that Clippy's pattern matching does see this any more.

[bluss]

IMO the changed implementation is a safety risk. Maybe I don't really want to analyze it, but I really don't think clippy should push us into this, it's not better, it just doesn't detect it.

[adamreichold]

Can you expand on why you think it is worse than the previous one? The semantics of

let mut v = Vec::with_capacity(size);
v.set_len(size);

are the same as

MaybeUninit::uninit().assume_init()

are they not?

[adamreichold]

but I really don't think clippy should push us into this

I would also say that Clippy pushes us into not having this method which we already know as it is deprecated for exactly this reason.

[adamreichold]

In any case, I would not mind dropping the commit as this does not affect beta yet and the method will probably go away soon in any case?

[bluss]

I don't have the overhead to analyse it again. It's probably the same like you said. Thus we don't need to change it :)

[adamreichold]

Replaced by a commit that just suppresses the lint for the deprecated method.

[adamreichold]

The remaining lints are missing_safety_doc in public-but-unimplementable traits. I don't think it is wise to global disable this lint, but I could suppress it in the relevant traits by manually verifying that they are indeed unimplementable downstream?

[adamreichold]

The remaining lints are missing_safety_doc in public-but-unimplementable traits. I don't think it is wise to global disable this lint, but I could suppress it in the relevant traits by manually verifying that they are indeed unimplementable downstream?

This leaves FixedInitializer and NdIndex which seem to be part of the crate API and implementable downstream so they really seem to be missing safety docs. Or should one or both of them be marked using the private_decl! marco?

[bluss]

I guess both of those traits are not really extension interfaces either. At least the former should disappear now with const generics.

[adamreichold]

I guess both of those traits are not really extension interfaces either. At least the former should disappear now with const generics.

Marked them using private_decl and private_impl.

One thing I wondered while adding those macro invocations is whether one could drop private_impl by providing a default implemenation in private_decl, e.g.

macro_rules! private_decl {
    () => {
        /// This trait is private to implement; this method exists to make it
        /// impossible to implement outside the crate.
        fn __private__(&self) -> crate::private::PrivateMarker {
            crate::private::PrivateMarker
        }
    }
}

[adamreichold]

error: unknown lint: `clippy::uninit_vec`
   --> src/impl_constructors.rs:614:13
    |
614 |     #[allow(clippy::uninit_vec)]
    |             ^^^^^^^^^^^^^^^^^^ help: did you mean: `clippy::unit_arg`
    |
    = note: `-D unknown-lints` implied by `-D warnings`

Ok, I will drop the commit preparing for uninit_vec... |-\

[bluss]

Thanks for working on this so eagerly, but we can't add private_decl in this PR, since that's a breaking change. Getting the clippy fixes in would be great before breaking changes.

[adamreichold]

Thanks for working on this so eagerly, but we can't add private_decl in this PR, since that's a breaking change. Getting the clippy fixes in would be great before breaking changes.

Ah right, this is strictly speaking a breaking change since they are indeed implementable downstream ATM... Will drop that commit as well.

[bluss]

Thanks!