jacobperron commented 3 years ago

This reverts commit 32942993e3f9afc8248be7b1f5944344fa2b4e30.

To unblock the buildfarm that's failing to build downstream packages transitively depending on this header being included.

jacobperron commented 3 years ago

Here's are failing jobs that this PR should fix:

There are probably several other jobs (that are blocked right now) that would have failed for the same reason.

mikepurvis commented 3 years ago

Could there be some kind of compat-type option here where we fix roscpp, and then in the header create our own aliases for _1, _2, etc, but with the [[deprecated]] attribute? This would resolve the warnings for the vast majority of cases while still signaling to the above packages that a deprecated usage exists.

jacobperron commented 3 years ago

@mikepurvis Thanks for the suggestion! This sounds okay to me. I'm happy to review a PR if you (or someone else) takes the time to implement it.

iwanders commented 2 years ago

Ok, to get back to this. I tried implementing @mikepurvis 's solution with something like;


#include <boost/bind/bind.hpp>

#define ROSCPP_BOOST_BIND_PROVIDER(name) \
[[deprecated("Roscpp used to provide the Bind placeholders (_1, _2, ...) in the global namespace is as boost provided them, this is deprecated. Please use <boost/bind/bind.hpp> + using namespace boost::placeholders, or define BOOST_BIND_GLOBAL_PLACEHOLDERS to retain the current behavior.")]]\
\
BOOST_STATIC_CONSTEXPR decltype(boost::placeholders::name) name;

ROSCPP_BOOST_BIND_PROVIDER(_1)
ROSCPP_BOOST_BIND_PROVIDER(_2)
ROSCPP_BOOST_BIND_PROVIDER(_3)
ROSCPP_BOOST_BIND_PROVIDER(_4)
ROSCPP_BOOST_BIND_PROVIDER(_5)
ROSCPP_BOOST_BIND_PROVIDER(_6)
ROSCPP_BOOST_BIND_PROVIDER(_7)
ROSCPP_BOOST_BIND_PROVIDER(_8)
ROSCPP_BOOST_BIND_PROVIDER(_9)
#undef ROSCPP_BOOST_BIND_PROVIDER

So effectively we don't have ros provide the global _1 anymore but we provide it from publisher.h and we include the boost/bind/bind.hpp which doesn't produce the warning.

The _1 is a static constexpr value, not a type. So if anyone now includes <boost/bind.hpp>, we have two instances of _1 in the global scope and the resolution is ambiguous. Specifically, this happens in signal9.h, which then uses the global placeholder _7 in this line, which is clearly a typo, but it shows the problem.

I thought of setting BOOST_BIND_NO_PLACEHOLDERS here to ensure we don't ever have boost populate them. But then the behaviour is suddenly dependent on the include order, which is not something that's desirable.

Basically, I think the only workaround is to just set BOOST_BIND_GLOBAL_PLACEHOLDERS to 1 to silence the warnings. We can do this from publisher.h, that way roscpp still provides the placeholders for the downstream packages, without having so much of this deprecation spam in the compile logs.

mikepurvis commented 2 years ago

Might it be possible to do it with a deprecated macro pragma instead, eg: https://stackoverflow.com/a/29297970/109517

But I guess fundamentally, we're probably not going to get a "real" fix for this given that it's only an issue on newer boosts than what is called out in REP-3 for Noetic.

iwanders commented 2 years ago

No, unfortunately a macro doesn't work, that doesn't do namespacing and breaks any using namespace;

Example:

namespace boost
{
namespace placeholders
{
  static constexpr int _1 = 1;
}
}

namespace std
{
namespace placeholders
{
  static constexpr int _1 = 5;
}
}

#define _1 _Pragma ("GCC warning \"'DEPRECATED_MACRO1' macro is deprecated\"") boost::placeholders::_1

int main()
{
  using namespace std::placeholders;
  return _1;
}

We should expect the std::placeholders to be returned here. However, we'll get the boost one; in the output of gcc -E;

int main()
{
  using namespace std::placeholders;
  return
# 22 "foo.cpp"

# 22 "foo.cpp"
  boost::placeholders::_1;
}

Compiling shows the warning & return code is 1 instead of 5.

hariharan82 commented 2 years ago

when i try to build ROS workspace in Raspberry Pi 400 (debian buster), i get the following error /home/pi/ros_catkin_ws/src/ros_comm/roscpp/include/ros/node_handle.h: In member function ‘ros::WallTimer ros::NodeHandle::createWallTimer(ros::WallDuration, void (T::*)(const ros::WallTimerEvent&), const boost::shared_ptr&, bool, bool) const’: /home/pi/ros_catkin_ws/src/ros_comm/roscpp/include/ros/node_handle.h:1436:74: error: ‘boost::placeholders’ has not been declared WallTimerOptions ops(period, boost::bind(callback, obj.get(), boost::placeholders::_1), 0); i find this page most relevant to the issue. i tried every possible solution I found online but non worked.

define BOOST_BIND_NO_PLACEHOLDERS and few others. Any help is much appreciated

jacobperron commented 2 years ago

@hariharan82 Please open questions like this on https://answers.ros.org , which is our central question-and-answers site.

ros / ros_comm

Revert "Fix warning related to Boost bind placeholders declared in global namespace. (#2169)" #2187

define BOOST_BIND_NO_PLACEHOLDERS and few others. Any help is much appreciated