Remove 'using namespace' from condition_variable.h

[emersonknapp]

Modify #1932 to not have using namespace in the header. It was causing downstream build failures in a project that included a library that did using namespace std::chrono; in its header at the global scope. That's terrible practice that this library has done such a thing, but the build had been working before #1932, so this fix will help our package and harm no one.

[emersonknapp]

@meyerj @dirk-thomas requesting review

[dirk-thomas]

Thanks for the patch.

[meyerj]

Thanks for the patch.

I assumed that using namespace boost::chrono at local scope (inside a function body) would always be fine, even in a header file, and that it has a higher precedence than using directives at namespace scope, but obviously that is wrong. The motivation was to minimize the diff to the condition_variable implementation from Boost.

[emersonknapp]

The problem wasn't really about precedence - it's about whether a name can be resolved to a single definition. A simplified example:

#include <cstdio>


namespace A {
  void ambiguous_fn() {
    printf("A::ambiguous_fn\n");
  }
}

namespace B {
  void ambiguous_fn() {
    printf("B::ambiguous_fn\n");
  }
}

// Somebody has polluted the global namespace.
// In my case, a header from a different library did this with "using namespace std::chrono"
using namespace A;

void ambiguity() {
  // This doesn't affect the namespace outside this function
  using namespace B;
  // The compiler doesn't know if you mean A::ambiguous_fn or B::ambiguous_fn, it has no way to tell
  // This function is not at fault but it can make sure that there is no confusion by using fully qualified name instead
  ambiguous_fn();
}

The problem is that the header is affected by the global namespace - a .cpp compilation unit would not have caused an issue here. Basically the moral of the story is that you probably shouldn't ever using namespace in a header because you can't know what the user's global namespace looks like by the time they get to your header.

Anyways, we're all good now. Cheers!

[clalancette]

For reasons I can't fully explain, this is breaking downstream consumers. One example is http://build.ros.org/view/Mbin_uB64/job/Mbin_uB64__grid_map_rviz_plugin__ubuntu_bionic_amd64__binary/37/console , where wait_until can't be resolved.

It looks like somehow the using boost::chrono was bringing an implementation of ceil into scope, and that is no longer the case.

I'm going to be posting a patch presently which fully qualifies the ceil and adds #include <cmath> to ensure it is available.

[clalancette]

For reasons I can't fully explain

Oh, I guess there is a boost::chrono::ceil. That explains it.