allwpilib
allwpilib copied to clipboard
Increase constexpr support in geometry data types
Makes geometry primitives (Pose2d, Pose3d, etc) more usable in constexpr context. This is useful for using these types as compile-time constants for path waypoints, measurements, and configurations. Tests are included to ensure constructors and functions can be evaluated at compile time.
Only significant implementation change is that Sin()
, Cos()
, and Tan()
are lazy-evaluated in frc::Rotation2d
so the angle can be used in constexpr context to support compile-time construction of frc::Pose2d
. This should have no impact to public API.
The main limitation to making more functions and classes constexpr is the limited constexpr math capabilities. cmath
changes have been proposed, but this would be C++23 or later. Alternative compile-time math libraries exist, but it would be best to only adopt these once std::is_constant_evaluated
could be used to choose the compile-time evaluated implementation only when running in a constexpr context.
I think clang-tidy is failing due to some #include statements missing from the new .inc files.
Any recommendation on a clean fix? Seems like a messy solution to include in the .inc files and the .h files even though it should work.
We already add #includes to .inc files for other classes, so we're fine with it.
We want to upgrade to GCC 11 or 12 on Athena for 2023, which would give us access to std::is_constant_evaluated()
and make constexpr Rotation2d cleaner. The earliest versions that have it are GCC 9, Clang 9, and MSVC 2019 v16.11.14 or MSVC 2022 v17.2.
Great! I'd be happy to revisit this once the new compiler versions are available. Rotation2d is most reliant on these changes in the 2d geometry classes. I didn't really touch 3d due to the can of worms eigen opens.
Yea... Eigen is making some stuff constexpr nowadays, but they're waiting on std::is_constant_evaluated()
as well.
I tried making a constexpr Matrix class when working on the CoordinateSystem class to make it and all the 3D geometry classes constexpr, but it was buggy and didn't have API parity with Eigen (some function overloads should only work with Vector). I assume they implemented that with template specializations and derived classes of MatrixBase.
Maybe we can revisit Rotation2d at a later date, and get Translation and Transform in now?
/azp run
Azure Pipelines successfully started running 1 pipeline(s).
Transform2d uses Rotation2d, so the only thing we can make constexpr without changing Rotation2d is the Translation2d constructor.
I don’t like the runtime pessimization of this change. It adds a conditional branch and codegen to every operation and increases the size of the structure by 8 bytes.
We could wait for the roboRIO compiler upgrade and C++20 to use std::is_constant_evaluated(). What's the timeline on that?
I’m thinking the upgrade will happen this summer?
I'm fine with delaying this change until after the compiler and C++ standard upgrade. The rotation2d implementation change to work with C++17 is definitely suboptimal.
Now that #4239 was merged, is this still blocked?
Not blocked, I just haven't gotten to updating this PR with the C++20 content yet