Implements rint

[khurd21]

Functions implemented:

• ccm::rint(float)

• ccm::rint(double)

• ccm::rint(long double)

• ccm::rintl(long double)

• ccm::rintf(float)

• ccm::lrint(float)

• ccm::lrint(double)

• ccm::lrint(long double)

• ccm::lrintl(long double)

• ccm::lrintf(float)

• ccm::llrint(float)

• ccm::llrint(double)

• ccm::llrint(long double)

• ccm::llrintl(long double)

• ccm::llrintf(long double)

Tests written for:

• Common and edge case conditions for float, double, and long double inputs.
• Assert that FE_INVALID is raised for +/- infinity values and NaN values.
• Asserts that functions can be evaluated at compile time.

[Rinzii]

Glad to see a new PR! I will warn you though. I've recently pushed a major internal restructure of the project to the dev branch so make sure your fork is up to date!

[khurd21]

@Rinzii

I noticed two things from your comments that seem the most important. I wanted to write them down here because I don't think I addressed these in an earlier PR for implementing ccm::nearbyint.

• constexpr keyword being used to get the rounding mode: I used constexpr for ccm::nearbyint implementation.
• Testing rounding mode type at runtime. I did not add these for nearbyint, however it does look like adding them just for rint would be testing the same behaviors.

Should I also re-address these two things there? I think for this PR, I could change constexpr to const in the ccm::nearbyint and leave it at that, however it makes sense to test the rounding modes in that function as well as it would have caught this potential bug in getting the rounding mode at runtime.

Thanks for the review!

[khurd21]

@Rinzii

I tried to address a few things in my recent changes:

• Removed the constexpr when getting the rounding mode to prevent compile time evaluation.
• Added a check for when FE_INVALID is raised to assert that it is being performed at runtime. To do this, I used ccm::support::is_constant_evaluated. Please do let me know if this was not was you were suggesting!

For addressing the different rounding modes, I was struggling to find a clean way to test them all without having a ton of duplicate code. I ended up making a decision to have cmake generate source files for us, changing the rounding mode for each source file generated. The end result is four generated files to test: DOWNWARD, TONEAREST, TOWARDZERO, and UPWARD. They all use the same template rint_test.cpp.in. This also forced me to separate out the tests that test for compile time evaluation. These are now in: rint_test_compile_time.cpp.

Here is an example of what they would look like in an IDE. The files are placed in an out folder for the specific compiler in use.

In doing so, I also found that the static_cast for the llrint, lrint etc functions seem to be a non-issue. The rounding for the floating point values are already performed before the static_cast in performed, so because they are representing a whole number at that point, a static cast should not have an adverse effect on the number.

I did notice though that msvc handles max and min values slightly differently for llrint and lrint functions. If it is the maximum or minimum, it returns 0. Clang and gcc did not seem to be implemented this way. If this sounds OK to you, I was considering adding a check to return 0 for MSVC given these conditions.

It also sounded like I may be going a different route with handling the rounding modes for the llrint-like functions, so I wanted to check in with your opinion before going much farther!

Thanks, and sorry about the slower responses on the PR!

[Rinzii]

Using cmake to generate different test cases is perfectly fine.

Also adding a msvc specific return is perfectly fine and actually encouraged as ccmath tends to aim to follow the expected output of each compiler!

Also don't worry about taking your time with the PR. I'd rather you took the time to get everything right than rushing stuff!

If you have any specific questions for me just lmk! I've been pretty busy these last few weeks, but I'll try to get back to any questions you have promptly.

[Rinzii]

Hey @khurd21 sorry for being somewhat away. Been going through quite a bit recently, but I have made a discord specifically for ccmath to allow quicker collaboration between contributors or users of the library. Feel free to join if you so wish!

Discord Link: https://discord.gg/p3mVxAbdmc

[Rinzii]

Also what is the current status of this PR if I may ask?

[khurd21]

Also what is the current status of this PR if I may ask?

Hey! Sorry I haven't touched this in a long time! The current status is I believe I need to update the code slightly to be compatible with how MSVC handles the edge cases compared to other compilers. I also wanted to double check that the test cases were accurate