alloc debug overflow on size 0 about rendy HOT 11 OPEN

Size and align 0 are not supported. Should add this to asserts as nd doc.

from rendy.

please don't assert if size is 0. just return an allocation error if possible, or at least do a checked sub and unwrap so it doesn't do weird stuff in release mode; but ideally just return an error and let the client decide if they want to crash the process?

from rendy.

I don't want to add another error variant. Especially since we use error type from gfx-hal.
Creating buffer of size 0 violates Vulkan API usage already and it typically happens before memory allocation so we already in dangerous waters where anything can happen.

Doing checking sub and unwrapping is much worse than meaningful assert IMHO.
You can't allocate 0 bytes not because of this expression (it can be modified to permit 0) so unwrapping there will only confuse the user.

from rendy.

ok, agreed assert better than debug overflow unwrap; even better it's caught even farther upstream, but yea.

Is creating a buffer of size 0 in vulkan really a validation error? I would assume it wouldn't care; it should be able to hand out an infinite amount of 0 size buffers ; )

from rendy.

Yes. Creating buffers of size 0 is invalid usage.

from rendy.

I think we can just add asserts in debug mode.

from rendy.

@omni-viral

Shouldn't the assert also be there in release? Since in release it will wrap and a bunch of unexpected things will happen?

from rendy.

If it’s invalid usage for size 0 simple panic with error message is best imho, and then documenting this as a panics condition :)

Asserts are for pre and post condition enforcement and author programmer errors

from rendy.

@m4b std uses asserts for invalid inputs like out of bounds for some Vec methods etc.
So I'd say assert is idiomatic way in Rust.

from rendy.

Out of bounds panics with bounds len violation message, last I checked. I’m sure there are asserts in vec but I can’t remember last time an assert violation bubbled up to the user but I could be mistaken.

from rendy.

In any event I mostly care about fixing the debug overflow which wasn’t super helpful wrt tracking down cause, and I don’t really care if it’s a panic or assert, just a better error message. This is effectively a one line fix we’re talking about so maybe we should just add it :)

from rendy.