search

boostorg / property_tree

Boost.org property_tree module
http://boost.org/libs/property_tree
55 stars 92 forks source link

Add missing includes #112

Closed ashtum closed 9 months ago

pdimov commented 10 months ago

It's hard to say what's the history here.

I suppose this dates from the days when including <boost/bind.hpp> made _1 et al available in the global namespace, which later conflicted with using namespace std::placeholders; which also made _1 available.

So I assume people who were defining BOOST_BIND_NO_PLACEHOLDERS did that in order to be able to use using namespace std::placeholders; in their code. But doing so broke parser.hpp.

This problem will not occur today, because <boost/bind/bind.hpp> leaves the placeholders defined in namespace boost::placeholders, avoiding the conflict. But for backward compatibility, we may want to still support the configuration with BOOST_BIND_NO_PLACEHOLDERS defined.

So adding this include is probably correct, but we need a test that shows why.

pdimov commented 10 months ago

As a general rule, every change that does not immediately fix a test failure needs to follow the procedure of: first commit adds a test that fails without the change, second commit adds the change that fixes the test failure.

pdimov commented 10 months ago

Would you please be able to extract the new test into a separate PR? I'd like to merge the original contribution (#101) as a form of giving credit on top of it.

ashtum commented 10 months ago

Then this PR is waiting for https://github.com/boostorg/property_tree/issues/100 response.