Fix build warnings on Windows #390
Conversation
These were caught as MSBuild warnings on Windows. In most cases, acknowledge that the conversions may lose precision by doing a static cast. In one case we change the type of an argument to be more accurate. Signed-off-by: Jacob Perron <[email protected]>
Fixes a MSBuild warning on Windows. Signed-off-by: Jacob Perron <[email protected]>
orocos_kdl/src/tree.cpp
Outdated
| @@ -156,7 +156,7 @@ bool Tree::getChain(const std::string& chain_root, const std::string& chain_tip, | |||
| } | |||
|
|
|||
| // add the segments from the common frame to the tip frame | |||
| for (int s=parents_chain_tip.size()-1; s>-1; s--){ | |||
| for (int s=static_cast<int>(parents_chain_tip.size())-1; s>-1; s--){ | |||
There was a problem hiding this comment.
Why not go for an unsigned int here?
There was a problem hiding this comment.
The condition for the loop is to check if s>-1, so it must be able to become negative.
There was a problem hiding this comment.
And if we check for s>=0?
There was a problem hiding this comment.
If s is an unsigned int it cannot be negative, so s-- will cause an integer overflow and this will become an infinite loop.
I've updated the code to use a reverse iterator, which I think makes it more readable (3324420).
| @@ -61,7 +61,7 @@ class TreeIkSolverPos_Online: public TreeIkSolverPos { | |||
| * @return | |||
| */ | |||
|
|
|||
| TreeIkSolverPos_Online(const double& nr_of_jnts, | |||
| TreeIkSolverPos_Online(const unsigned int& nr_of_jnts, | |||
There was a problem hiding this comment.
This is an API breaking change. This has been discussed in #384. So I think we should skip this one.
There was a problem hiding this comment.
Updated to avoid breaking API: d70e705
There was a problem hiding this comment.
Not sure why, but it looks like this change (d70e705) broke a test 🤔
There was a problem hiding this comment.
Since most of the members we were initializing with this value where being overridden immediately in the constructor, I've just removed them from the initializer list (fc0dfd8). CI looks happy now.
Use static cast instead to avoid compiler warnings. Signed-off-by: Jacob Perron <[email protected]>
They are set inside the constructor anyways. Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
|
I guess CI is flaky? Since the same change passed CI on my fork: https://github.com/jacobperron/orocos_kinematics_dynamics/runs/5796424695?check_suite_focus=true |
Yes, see #239 |
We noticed these warnings when building orocos_kdl as part of ROS 2 CI, see https://ci.ros2.org/job/ci_windows/16708/msbuild
I've triggered a new Windows build with the changes in this PR: