NFD » ndn-cxx

I prefer the solution of specializing std::is_placeholder. Although it causes Boost placeholders to be used everywhere, this solution does not need using namespace std::placeholders, and still allows writing each placeholder as _1 shortcut.

We don't have conflicts right now. Generally, I do not quite like forcing boost placeholders, where std ones should have been used.

Given we are using c++11, in some cases it is easier to rewrite code using lambda functions, then to play with bind and placeholders.

I hit a conflict in a program that is not within namespace ndn, and has a using namespace ndn; statement.
Whenever _1 is used in that code, gcc 4.8 complains that it's ambigious.

If we want to use std placeholders, we should adopt the first workaround.

Is the program itself includes boost or boost is implicitly included from ndn-cxx headers?

The first workaround could be acceptable, though I would prefer having this define only around boost includes in ndn-cxx header files. In other words, if the program itself uses boost, it should define BOOST_BIND_NO_PLACEHOLDERS itself to avoid the conflict. We shouldn't force this property to the apps that just depend on ndn-cxx.

I constructed a minimal program to reproduce this issue:

#include <functional>
//#define BOOST_BIND_NO_PLACEHOLDERS
#include <ndn-cxx/security/validator-config.hpp>

using namespace ndn;
using namespace std::placeholders;

void
f(int i)
{
}

int
main()
{
  bind(f, _1);
  return 0;
}

Build error is:

shijunxiao@m0212:~/snippet$ g++ --std=c++0x x.cpp `pkg-config --cflags --libs libndn-cxx`
x.cpp: In function ‘int main()’:
x.cpp:16:11: error: reference to ‘_1’ is ambiguous
/usr/include/c++/4.6/functional:859:34: error: candidates are: const std::_Placeholder<1> std::placeholders::_1
/usr/include/boost/bind/placeholders.hpp:55:15: error:                 boost::arg<1> {anonymous}::_1

The troublemaker is indeed <boost/property_tree/ptree.hpp>.

It seems that solution 2 isn't feasible in this case.
Uncommentting #define BOOST_BIND_NO_PLACEHOLDERS gives build errors in multi-index, which expects _1 to be available globally.

However, having both #define BOOST_BIND_NO_PLACEHOLDERS and using namespace std::placeholders; (in global namespace, not in ndn) before Boost includes allows the program to compile correctly.
But having using namespace in header and in global namespace is evil.

For the library, potential workaround would be to hide all boost related things using implementers approach. The app shouldn't really import both ndn and std::placeholders namespace into the program. There are could be other legitimately conflicting things if one does so.

heh... I told you this was going to explode sooner or later...

Our library MUST NOT mandate any specific usage of boost and std. The fact that boost imports _1 into the global namespace should not be our concern, as we never rely on this in the library code. If some application imports ndn namespace, it should take care of global _1.

The following snippet compiles without including the library and fails when library (commit 02adbb5f4c12d8fb3f5f439019dd7b758a2d90dd) is included:

// #include <ndn-cxx/common.hpp>

#include <boost/bind.hpp>
#include <iostream>

void
doSomething1(int i)
{
  std::cout << i << std::endl;
}

int
main(int argc, char** argv)
{
  auto x1 = boost::bind(&doSomething1, _1);
  x1(1);

  return 0;
}

I would agree to include the define to disable import global import of the boost placeholder. But this macro should only surround boost includes in all headers. It should not exist outside the header files.

Alex Afanasyev wrote:

Our library MUST NOT mandate any specific usage of boost and std. The fact that boost imports _1 into the global namespace should not be our concern, as we never rely on this in the library code. If some application imports ndn namespace, it should take care of global _1.

This is reasonable, however I don't understand why we shouldn't make the library more foolproof and easier to use if we can.

// #include <ndn-cxx/common.hpp>

I thought we declared common.hpp as internal, i.e. it must not be included in user code.

I would agree to include the define to disable import global import of the boost placeholder. But this macro should only surround boost includes in all headers. It should not exist outside the header files.

So basically you're proposing the following...?

#define BOOST_DONT_MESS_WITH_PLACEHOLDERS
#include <boost/broken-header.hpp>
#undef BOOST_DONT_MESS_WITH_PLACEHOLDERS

Davide Pesavento wrote:

Alex Afanasyev wrote:

Our library MUST NOT mandate any specific usage of boost and std. The fact that boost imports _1 into the global namespace should not be our concern, as we never rely on this in the library code. If some application imports ndn namespace, it should take care of global _1.

This is reasonable, however I don't understand why we shouldn't make the library more foolproof and easier to use if we can.

// #include <ndn-cxx/common.hpp>

I thought we declared common.hpp as internal, i.e. it must not be included in user code.

At least as it stands now, it is pulled from every other include. The same code snippet would fail when anything else from the library is included.

I would agree to include the define to disable import global import of the boost placeholder. But this macro should only surround boost includes in all headers. It should not exist outside the header files.

So basically you're proposing the following...?

#define BOOST_DONT_MESS_WITH_PLACEHOLDERS
#include <boost/broken-header.hpp>
#undef BOOST_DONT_MESS_WITH_PLACEHOLDERS

Yes. It should be a little bit more complicated, with additional check if the macro is already defined, and the macro should not be undefined if it was defined before. But essentially, yes. If we have these extra things, I will be happy with the fix.

I'm trying with having #define BOOST_BIND_NO_PLACEHOLDERS only around Boost includes in an ndn-cxx header as commit 5fa7fae44a54736e6b55b694bbfb58dc500c1961, however it doesn't compile, because Boost Multi-Index Container expects global _1.

It seems that we'll have to:

specialize std::is_placeholder according to https://svn.boost.org/trac/boost/ticket/2240#comment:10

Commit b72e31388f409720325f9b1de913e25b426734cd has the alternate solution from https://svn.boost.org/trac/boost/ticket/2240#comment:10 .
Both test snippets can compile successfully.

Hm... Ok. I could change my mind, as things getting complicated. I even think that there is no solution that can solve all the problems. Solutions will solve some problems, but there still be cases when error can be generated.

Ideally, all headers that generate such conflicts need to be hidden in the implementation. This would avoid the problem in the first place. For now, we could disable global import of boost placeholders, but we should clearly document how to fix problems if users encounter them.

Commit a958923fe371809a0007eace399f45c228b4a205: I finally found a solution that works for both gcc and clang.