vulkano-rs / vulkano

Safe and rich Rust wrapper around the Vulkan API
Apache License 2.0
4.45k stars 435 forks source link

Enable and fix pointer cast lints #2507

Closed Rua closed 4 months ago

Rua commented 4 months ago

This enables and fixes the following Clippy lints:

I think there should be no more raw pointer casts left using as at all now, they should all be explicitly marked as above.

MarijnS95 commented 4 months ago

Nice stuff, I wish I was allowed to use this more rigorously in my crates :)

ref_as_ptr is the only weird one of the bunch, relying on coercion seems the nicest to me, typically of the form <*const _>::cast(&foo) :)

marc0246 commented 4 months ago

I also prefer <*const T>::cast(&x). There are several instances of me doing that in the project already. The nice thing is that it doesn't require the latest stable.

MarijnS95 commented 4 months ago

Yup, and coercion already disallows turning a & in a *mut (without as casts).

Rua commented 4 months ago

Where is that cast function defined? Is it just the pointer method?

marc0246 commented 4 months ago

It's the fully-qualified syntax to get at the inherent method of the pointer type, yes.

Rua commented 4 months ago

And yet, you're passing it a reference? Surprised that just works.

marc0246 commented 4 months ago

Yes, it's the auto-coersion from reference to pointer as Marijn pointed out.

MarijnS95 commented 4 months ago

Yeah cast() is only available on pointer types. Instead of doing (&foo as *const T).cast(), which results in a clippy lint saying that the as cast can be performed via coercion, we "specify" the left-hand type with the fully-qualified syntax to refer to the cast method on type *const T. The alternative let foo: *const T = &foo; ... foo.cast() might also work, but is typically more obtrusive.

Rua commented 4 months ago

I left a few instances of .map(ptr::from_ref). If you'd prefer to replace those too, to bring down the MSRV, I can do that.

marc0246 commented 4 months ago

I would certainly prefer <*const _>::cast(&x) over ptr::addr_of!(x).cast() as well for the sake of consistency. Otherwise I love it!

MarijnS95 commented 4 months ago

Just want to say, a lot of changes to code that will disappearâ„¢ when moving to ash' .push_next() lifetime-safe builders :)

marc0246 commented 4 months ago

Yeah haha :3