Do not pass and return simple types by const ref

[Shobuj-Paul]

Description

Fixes #859
Continues the work from #2119

Checklist

• Required by CI: Code is auto formatted using clang-format
• Extend the tutorials / documentation reference
• Document API changes relevant to the user in the MIGRATION.md notes
• Create tests, which fail without this PR reference
• Include a screenshot if changing a GUI
• While waiting for someone to review your request, please help review another open pull request to support the maintainers

[codecov]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (faa4795) 50.35% compared to head (a5eb887) 50.83%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2453      +/-   ##
==========================================
+ Coverage   50.35%   50.83%   +0.49%     
==========================================
  Files         390      391       +1     
  Lines       31954    32125     +171     
==========================================
+ Hits        16087    16328     +241     
+ Misses      15867    15797      -70     

Files	Coverage Δ
...include/moveit/robot_trajectory/robot_trajectory.h	98.67% <ø> (ø)
...eit_core/robot_trajectory/src/robot_trajectory.cpp	66.86% <100.00%> (ø)
...de/moveit/ompl_interface/detail/ompl_constraints.h	100.00% <100.00%> (ø)
...z_industrial_motion_planner/trajectory_generator.h	100.00% <100.00%> (ø)
...ustrial_motion_planner/trajectory_generator_circ.h	100.00% <ø> (ø)
...dustrial_motion_planner/trajectory_generator_lin.h	100.00% <ø> (ø)
...dustrial_motion_planner/trajectory_generator_ptp.h	100.00% <ø> (ø)
...strial_motion_planner/src/trajectory_functions.cpp	100.00% <ø> (ø)
...strial_motion_planner/src/trajectory_generator.cpp	96.76% <100.00%> (ø)
...l_motion_planner/src/trajectory_generator_circ.cpp	99.09% <ø> (ø)
... and 5 more

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Shobuj-Paul]

@tylerjw Please check if this is fine.

[ChrisThrasher]

In general this PR does good stuff. Changing const double& to double in function declarations is a good change. Performance wise it makes no difference since doubles are 64 bits and we're all using 64 bit computers, but it's certainly a bit odd to see these fundamentally types used via a const ref. Likewise returning a const ref to a built in type is weird and doesn't solve any problems, generally speaking. It's good to avoid such things.

Let's be careful though. By changing const double& to double in a function definition we now are opening up the ability to mutate data was previously immutable which means some amount of intent is lost and it's a bit easier for someone to make potentially invalid changes to the code in the future. While adding const to a non-reference parameter is meaningless in a function declaration, it does carry meaning in the function definition.

Also when changing const double& return values to double we run the risk of API breaks if users are storing the return value in another const double&. Let's keep that in mind in case any public functions are being changed.

const double& value = someMoveItFunction(); // This breaks when the return type is not longer a reference

[ChrisThrasher]

This has the the potential to change semantics since a reference to this double is prone to be changed by the calling code.

[Shobuj-Paul]

Reverted back.

[Shobuj-Paul]

I am not sure if this particular struct is part of the API, but it does seem accesible the user to call.
One thing I am confused about though, the function ContactTestData takes in a const double& parameter and initialises the variable. Was the function declared with nature of the reference in mind or was the variable declared just because it had to be assigned the const ref type passing in the function?
If it's the latter, maybe we should prefer to have it just as a double instead of a const&.

[ChrisThrasher]

No more comments to make. Dismissing the review request.