Conversation
ash/src/device.rs
Outdated
| &mut count, | ||
| out.as_mut_ptr(), | ||
| ); | ||
| assert_eq!(count, out.len() as u32); |
There was a problem hiding this comment.
What's the advantage of not being explicit about this (trivial to write) type?
That said, it's probably better to cast count to usize to prevent a possible truncation.
There was a problem hiding this comment.
I am always wondering wether to use as _ or be explicit...
There was a problem hiding this comment.
@filnet There are probably no real API guidelines but it's probably better to be explicit unless intentionally hiding long type casts.
I've also turned on the trivial-casts and trivial-numereric-casts default-allow rustc warning locally to notice that there are quite a few casts to the same type, mostly as _ which was copied verbatim without the programmer really knowing or caring about the source and target type.
In either case we should aim to flip that warning on and make the code ever so slightly more readable by removing stray as casts.
That said, it's probably better to cast
counttousizeto prevent a possible truncation.
On second thought I did this by intention to match the truncation from let mut count = out.len() as u32;. However, that means - if someone manages to pass a slice longer than u32::max at all - this assert won't fail even if it should. Erring on the side of correctness, this is now flipped.
Ralith
left a comment
There was a problem hiding this comment.
Built-in sanity-check asserts feels like it might be a bit opinionated compared to ash's status quo, but none of this is hotpath stuff anyway.
Originally introduced in [#489] this inserts the array-length equality check everywhere else: in the supposedly invalid and inexistant event where Vulkan suddenly returns less items (`count` has been modified) than were originally queried through respective `_len()` functions (or more likely: a slice of invalid length passed by the user) and some elements at the end of the slice are left uninitialized, panic. Wherever there is valid concern or possibility for this to happen - or to circumvent assertions and panics altogether - mutable references to mutable slices should be passed allowing the length to be promptly updated. [#489]: #489 (comment)
Originally introduced in #489 this inserts the array-length equality check everywhere else: in the supposedly invalid and inexistant event where Vulkan suddenly returns less items (
counthas been modified) than were originally queried through respective_len()functions (or more likely: a slice of invalid length passed by the user) and some elements at the end of the slice are left uninitialized, panic.Wherever there is valid concern or possibility for this to happen - or to circumvent assertions and panics altogether - mutable references to mutable slices should be passed allowing the length to be promptly updated.