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

remove force_eval! macro, prepare more const functions, fix clippy, and add clippy to CI #288

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

phip1611
Copy link
Contributor

@phip1611 phip1611 commented Dec 18, 2023

This MR is intended to be a preparation for #262.

  • fix all clippy warnings (or properly ignore, where sensible)
  • add clippy to CI
  • remove the force_eval! macro with something more const friendly

Regarding the removal of the force_eval!-macro:
Fomment from @Amanieu from the other PR:

I am generally in favor of the PR!

The volatile reads are a bit of a hack. I believe they were used for 2 purposes:

  • To set the appropriate FP status flags (overflow, underflow, etc) in the FP status register. We really shouldn't bother with this since we don't guarantee any particular value for flags.
  • To work around FP precision issues on 32-bit x86 which uses 80-bit float precision internally. This is unfortunately an unavoidable issue on x86 since it has pretty broken FP support.

Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
Now, many functions can become const functions.

Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
@phip1611
Copy link
Contributor Author

phip1611 commented Dec 18, 2023

ping @Amanieu - I'm not sure whether the CI error might be unrelated.

@Amanieu
Copy link
Member

Amanieu commented Dec 19, 2023

The test failures don't happen on master, this is definitely related to your changes.

@Amanieu
Copy link
Member

Amanieu commented Dec 19, 2023

You can reproduce the test failures locally with cargo test --release.

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.

None yet

2 participants