Revert PR #197 and re-implement to fix incorrect buffer writes

[jaynus]

#197 unintentionally allowed for incorrectly writing Vec types when collected and submitted. This was discussed in the PR and actually seen, where a crash was mentioned, but it was incorrectly assumed to be an upstream problem. The change in #197 caused upload_* functions to accept a plain Vec as &T can AsRef a Vec, which was then cast incorrectly to a u8 and submitted.

This was only evident in the quads example because it is the only example which actually provides a Vec type (from a Vec::collect call) for a buffer upload.

This PR reverts #197 and changes the fix for #58 to the original concept of just bounding T: 'static + Copy, instead of changing from &[T] to &T. As discussed in #197, this will have problems with large arrays being submitted - but we can just ignore that, as its unrealistic; it is better to correctly accept Vec types for large arrays, instead of compensating for large static slices which will almost never exist.

[codecov-io]

Codecov Report

Merging #219 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #219   +/-   ##
=======================================
  Coverage   87.57%   87.57%           
=======================================
  Files           6        6           
  Lines         169      169           
=======================================
  Hits          148      148           
  Misses         21       21

Impacted Files	Coverage Δ
mesh/src/mesh.rs	92.3% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90ea766...7190057. Read the comment docs.

[Frizi]

The odd arrays problem is not big in practice as any array size can be converted to slice, and slice of arrays can be converted into a flat slice as well. The only case where this could be a problem is custom structs with oddly sized slices inside, but even that can be worked around by converting into &[u8], which is OK for all Copy (or should-be copy) types.

[zakarumych]

Passing Vec should be impossible due to !Drop check implemented via static assert on needs_drop which is const fn. IDK how we missed that in #197

[zakarumych]

About slices:

• Pros: coercion.
• Cons: cannot use with ?Sized types and force wrapping singular values using slice_from_ref

[Frizi]

I think having ?Sized types that are not slice is extremally rare in this context. And even then, you can likely just cast it to byte slice anyway. We could use Read trait maybe, but I think keeping things simple as long as we don't have a valid reason not to is a good idea.

Having Copy is really what the intention is here. Not doing it because compiler is not smart enough today (and that's just for relatively rare cases) is not really a way to go. We do have workarounds for everything, you can still break it if you try hard enough, but it is indeed harder and makes errors stand out. Alternative we have now is that very subtly invalid code compiles without warning and triggers a hardware/drivers dependant crash that often takes the whole system down.

[zakarumych]

Alternative we have now is that very subtly invalid code compiles without warning and triggers a hardware/drivers dependant crash that often takes the whole system down.

That's why I suggest asserting that type is !Drop.
I know that Copy is the right thing here. If only std and built-in types would implement it when they should.

[Frizi]

From needs_drop docs:

This is purely an optimization hint, and may be implemented conservatively: it may return true for types that don't actually need to be dropped. As such always returning true would be a valid implementation of this function.

https://doc.rust-lang.org/std/mem/fn.needs_drop.html

[jaynus]

This is purely an optimization hint, and may be implemented conservatively: it may return true for types that don't actually need to be dropped.

This doesn't really seem like something to reliably static assert on. It seems the Copy bound is the best route; it may force a small amount of extra effort on the user for a few cases; but at least we are correctly implementing the safety and not silently failing.

[zakarumych]

Ok. Good points.

[Frizi]

bors r=frizi,omni-viral

[bors]

Build succeeded

• continuous-integration/jenkins/branch