search

boostorg / unordered

Boost.org unordered module
http://boost.org/libs/unordered
Boost Software License 1.0
62 stars 55 forks source link

Fix/remove lifted ops #218

Closed joaquintides closed 10 months ago

joaquintides commented 10 months ago

Opening the PR for discussion. I've removed all lifting using directives for swap and operator(==|!=), except in the case of boost::unordered_[multi](map|set), and I wonder if we should go ahead and remove those too --I can't think of any legitimate use case where they could be needed.

pdimov commented 10 months ago

We should remove the 'lifting' for operator== and operator!= because only test suites do boost::operator==(x, y) instead of the correct x == y. For swap, things are less clear because we had a function boost::swap in Core so it's quite possible for code to have done boost::swap(x, y) for unordered_set or unordered_map.

If we remove the using directive, this will probably stop compiling, or start doing who knows what.

joaquintides commented 10 months ago

If we remove the using directive, this will probably stop compiling, or start doing who knows what.

From what I can gather from boost::swap reference, ADL should kick in, so it seems safe to me to drop the lift. I've tested the following locally (on the fix/remove_lifted_ops branch):

#include <boost/unordered/unordered_flat_map.hpp>
#include <boost/core/swap.hpp>

int main()
{
#pragma warning(disable : 4996)

  boost::unordered_flat_map<int,int> fm1,fm2;
  boost::swap(fm1,fm2);
}

and everything works fine. (BTW, the docs for boost::swap do not indicate that the function is deprecated.)

pdimov commented 10 months ago

It won't work if boost/core/swap.hpp isn't included, though.

codecov[bot] commented 10 months ago

Codecov Report

Merging #218 (f4667f5) into develop (ea4fc5e) will increase coverage by 0.01%. Report is 1 commits behind head on develop. The diff coverage is n/a.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/boostorg/unordered/pull/218/graphs/tree.svg?width=650&height=150&src=pr&token=ZqRPZlJZ5N&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg)](https://app.codecov.io/gh/boostorg/unordered/pull/218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) ```diff @@ Coverage Diff @@ ## develop #218 +/- ## =========================================== + Coverage 98.07% 98.08% +0.01% =========================================== Files 143 143 Lines 19693 19798 +105 =========================================== + Hits 19314 19419 +105 Misses 379 379 ``` | [Files](https://app.codecov.io/gh/boostorg/unordered/pull/218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) | Coverage Δ | | |---|---|---| | [include/boost/unordered/concurrent\_flat\_map.hpp](https://app.codecov.io/gh/boostorg/unordered/pull/218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg#diff-aW5jbHVkZS9ib29zdC91bm9yZGVyZWQvY29uY3VycmVudF9mbGF0X21hcC5ocHA=) | `100.00% <ø> (ø)` | | | [include/boost/unordered/concurrent\_flat\_set.hpp](https://app.codecov.io/gh/boostorg/unordered/pull/218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg#diff-aW5jbHVkZS9ib29zdC91bm9yZGVyZWQvY29uY3VycmVudF9mbGF0X3NldC5ocHA=) | `100.00% <ø> (ø)` | | | [test/unordered/compile\_tests.hpp](https://app.codecov.io/gh/boostorg/unordered/pull/218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg#diff-dGVzdC91bm9yZGVyZWQvY29tcGlsZV90ZXN0cy5ocHA=) | `100.00% <ø> (ø)` | | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/boostorg/unordered/pull/218/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/boostorg/unordered/pull/218?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/boostorg/unordered/pull/218?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg). Last update [ea4fc5e...f4667f5](https://app.codecov.io/gh/boostorg/unordered/pull/218?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg).