-
Notifications
You must be signed in to change notification settings - Fork 617
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
[wpimath] Make controllers and some trajectory classes constexpr #7343
[wpimath] Make controllers and some trajectory classes constexpr #7343
Conversation
9410c23
to
7f2837a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This moves a lot of functions to the headers without making them constexpr--what's the reason for this?
@@ -66,7 +109,14 @@ class WPILIB_DLLEXPORT Trajectory { | |||
* | |||
* @throws std::invalid_argument if the vector of states is empty. | |||
*/ | |||
explicit Trajectory(const std::vector<State>& states); | |||
explicit Trajectory(const std::vector<State>& states) : m_states(states) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were moved to the header but are not constexpr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They originally were, but we need GCC 12 for constexpr std::vector, which caused CI to fail. Bumping our RPi OS version to bookworm would fix that.
In any case, the functions are short enough to be inlined.
Does this have a measurable effect on build times? |
7f2837a
to
99bd370
Compare
99bd370
to
e3a68eb
Compare
Seems negligible. I just did a clean build with ccache cleared. main:
this PR:
(I have to skip failed compilations cuz cscore can't compile against my machine's version of OpenCV Java due to class file incompatibility.) |
No description provided.