abseil/abseil-cpp

Absl containers don't have a free swap method

Opened this issue · 2 comments

Describe the issue

A typical idiom from the copy & swap idiom is to do sth like this:

class MyClass {
    int i;
    std::unordered_set<int> std_set;
    absl::flat_hash_set<int> absl_set;

    // ...
    friend void swap(MyClass& m1, MyClass& m2) noexcept {
        using std::swap; // default
        swap(m1.i,        m2.i);        // std::swap(T&,T&)
        swap(m1.std_set,  m2.std_set);  // std::swap(std::unordered<int>&, std::unordered<int>&)
        swap(m1.absl_set, m2.absl_set); // std::swap(T&, T&) instead of m1.swap(m2)
    }

};

But Absl containers don't have a free swap method. So the code above picks the default std::swap(T&, T&) implementation instead of using m2.swap(m2).

Adding sth like this resolves the issue:

template</*...*/>
class flat_hash_set {
    // ...
    
    // will be found due to ADL
    friend void swap(flat_hash_set</*...*/& set1, flaht_hash_set</*...*/>& set2) noexcept {
        set2.swap(set2);
    }
};

Steps to reproduce the problem

see above

What version of Abseil are you using?

lts_2023_08_02

What operating system and version are you using?

Linux zen 6.5.11-1-MANJARO #1 SMP PREEMPT_DYNAMIC Thu Nov 9 02:34:53 UTC 2023 x86_64 GNU/Linux

What compiler and version are you using?

Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/13.2.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure --enable-languages=ada,c,c++,d,fortran,go,lto,objc,obj-c++ --enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --with-build-config=bootstrap-lto --with-linker-hash-style=gnu --with-system-zlib --enable-__cxa_atexit --enable-cet=auto --enable-checking=release --enable-clocale=gnu --enable-default-pie --enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object --enable-libstdcxx-backtrace --enable-link-serialization=1 --enable-linker-build-id --enable-lto --enable-multilib --enable-plugin --enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch --disable-werror
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.2.1 20230801 (GCC) 

What build system are you using?

cmake version 3.27.7

Additional context

No response

friend void swap(raw_hash_set& a,
raw_hash_set& b) noexcept(noexcept(a.swap(b))) {
a.swap(b);
}

void swap(raw_hash_set& that) noexcept(
IsNoThrowSwappable<hasher>() && IsNoThrowSwappable<key_equal>() &&
IsNoThrowSwappable<allocator_type>(
typename AllocTraits::propagate_on_container_swap{})) {

Is this not being found because std::allocator doesn't have propagate_on_container_swap? What problem is this causing?

Ok, I figured out the problem. Trouble is that the free swap functions are not in the same namespace as absl::flat_hash_set and, thus, ADL is not working as intended. Put another way:
We do have absl::container_internal::swap for absl::container_internal::raw_hash_set but we actually want to swap absl::flat_hash_set but there is not free swap function available in namespace absl. The other swap function is not considered for ADL.

My example above works as intended, if we add sth like this because now ADL sees a swap function in the same namespace as the type of absl::flat_hash_set:

namespace absl {
    template<class T, class H, class E, class A>
    void swap(absl::flat_hash_set<T, H, E, A>& a, absl::flat_hash_set<T, H, E, A>& b) noexcept(noexcept(a.swap(b))) {
    a.swap(b);
}
}