Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace type alias with a 'use'. #223

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

Vrixyz
Copy link
Contributor

@Vrixyz Vrixyz commented Jul 15, 2024

Fixes #222

image

@Vrixyz Vrixyz requested a review from sebcrozet July 15, 2024 09:55
src/lib.rs Show resolved Hide resolved
Copy link
Member

@sebcrozet sebcrozet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find, thanks!

We should consider doing that for the other aliases, e.g. use na::Vector3 as Vector instead of type Vector<N> = Vector3<N> (can be in a separate PR).

+1 for adding a changelog line.

@Vrixyz
Copy link
Contributor Author

Vrixyz commented Jul 18, 2024

We should consider doing that for the other aliases, e.g. use na::Vector3 as Vector instead of type Vector<N> = Vector3<N> (can be in a separate PR).

  • See Explore more controversial "use"s for types #232
    • the difference is more subtle and I'm not sure it's more consistent, it still redirects to an alias (which is alright, but has this big dislaimer "Because this is an alias, not all its methods are listed here. See the Point type too." ; but yes let's discuss it in the other PR.

@Vrixyz Vrixyz merged commit f305467 into dimforge:master Jul 18, 2024
5 checks passed
@Vrixyz
Copy link
Contributor Author

Vrixyz commented Jul 23, 2024

This adds a warning when running cargo +nightly clippy:

warning: casting to the same type is unnecessary (`f32` -> `f32`)
    --> crates\parry3d\../../src\shape\shape.rs:1466:9
     |
1466 |         f32::MAX as Real
     |         ^^^^^^^^^^^^^^^^ help: try: `f32::MAX`
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

Interestingly, this first one seems a good warning, we probably want Real::MAX ^ ; but the following is more difficult to fix:

warning: casting float literal to `f32` is unnecessary
  --> crates\parry3d\../../src\query\sat\sat_cuboid_cuboid.rs:14:18
   |
14 |     let signum = (1.0 as Real).copysign(pos12.translation.vector.dot(axi... 
   |                  ^^^^^^^^^^^^^ help: try: `1.0_f32`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

@waywardmonkeys
Copy link
Contributor

Real::MAX was what I intended to do when I got to these.

For the other, you could try Real::from(1.0) and see if that warns ... or a quick trait like the One trait from num.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to "use" instead of alias pattern for Real
3 participants