-
Notifications
You must be signed in to change notification settings - Fork 498
Use boost/bind/bind.hpp to fix warnings on Arch Linux #3156
Use boost/bind/bind.hpp to fix warnings on Arch Linux #3156
Conversation
The boost/bind.hpp file recommends switching to boost/bind/bind.hpp to avoid a legacy syntax for placeholders. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
There was a compilation issue with boost/bind/bind.hpp in EditorView.cc, and it was easy to fix it by switching to std::bind with std::placeholders::_1. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Use narrow scoping when possible, except in model_database.cc integration test. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Looks good to me -- and it's a more sensible fix than my hacky workaround (PR #3147). I tried build-testing it and I think the warnings were fixed although compilation ultimately failed because of #2867. Given the nature of the change though, I think as long as it passes your own CI checks then it should be fine. My one suggestion is that you might consider using either the boost version of (And FYI I've been testing things on Arch Linux with boost v1.78.0.) |
I agree it would be nice to be consistent with |
Fair enough. I can see that that wouldn't be a priority. Thanks for tackling my issue! |
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
I just noticed that a few files were including boost/bind.hpp but not calling |
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.
looks good to me.
This is probably expected and nothing can be done for it, but note that this could create several downstream compilation errors. I got robotology/gazebo-yarp-plugins#606, but I guess similar errors will appear in |
I hadn't thought about that aspect of it. I may revert this... |
Apologies, I only just saw this. It's annoying that the change breaks downstream code which relies on the global namespace being polluted with |
we aren't planning a gazebo12 release, but if we do then this would be a reasonable change to make |
It was previously reported in #2757 that there were numerous compiler warnings complaining about using boost bind placeholders (like
_1
) in the global namespace, since that practice is deprecated. This was fixed in #2809 for purposes of our CI machines, but as noted in #3147 it is still an issue for some users.This takes an alternative approach to #3147 by avoiding use of the
boost/bind.hpp
header file in favor ofboost/bind/bind.hpp
, as recommended in boost/bind.hpp itself. This worked directly for all files exceptgazebo/gui/building/EditorView.cc
, which had some arcane compiler errors. Since it was only one file; I migrated it to usestd::bind
withstd::placeholders::_1
in 4614ef9.cc @alexdewar