¿Está bien definir una función swap() totalmente general?

8 minutos de lectura

avatar de usuario
Tavian Barnes

El siguiente fragmento:

#include <memory>
#include <utility>

namespace foo
{
    template <typename T>
    void swap(T& a, T& b)
    {
        T tmp = std::move(a);
        a = std::move(b);
        b = std::move(tmp);
    }

    struct bar { };
}

void baz()
{
    std::unique_ptr<foo::bar> ptr;
    ptr.reset();
}

no me compila:

$ g++ -std=c++11 -c foo.cpp
In file included from /usr/include/c++/5.3.0/memory:81:0,
                 from foo.cpp:1:
/usr/include/c++/5.3.0/bits/unique_ptr.h: In instantiation of ‘void std::unique_ptr<_Tp, _Dp>::reset(std::unique_ptr<_Tp, _Dp>::pointer) [with _Tp = foo::bar; _Dp = std::default_delete<foo::bar>; std::unique_ptr<_Tp, _Dp>::pointer = foo::bar*]’:
foo.cpp:20:15:   required from here
/usr/include/c++/5.3.0/bits/unique_ptr.h:342:6: error: call of overloaded ‘swap(foo::bar*&, foo::bar*&)’ is ambiguous
  swap(std::get<0>(_M_t), __p);
      ^
In file included from /usr/include/c++/5.3.0/bits/stl_pair.h:59:0,
                 from /usr/include/c++/5.3.0/bits/stl_algobase.h:64,
                 from /usr/include/c++/5.3.0/memory:62,
                 from foo.cpp:1:
/usr/include/c++/5.3.0/bits/move.h:176:5: note: candidate: void std::swap(_Tp&, _Tp&) [with _Tp = foo::bar*]
     swap(_Tp& __a, _Tp& __b)
     ^
foo.cpp:7:10: note: candidate: void foo::swap(T&, T&) [with T = foo::bar*]
     void swap(T& a, T& b)

¿Es esto mi culpa por declarar un swap() función tan general que entra en conflicto con std::swap?

Si es así, ¿hay alguna manera de definir foo::swap() para que no sea arrastrado por la búsqueda de Koenig?

  • No se compila en GCC o Clang, pero se compila en MSVC 2015. Quizás otra característica no documentada.

    – Wally

    25/04/2016 a las 20:38

  • Maldita sea, esas son algunas buenas proporciones en menos de la primera hora. +16 votos a favor, visto 77 veces y 4 favoritos.

    –Mateen Ulhaq

    25/04/2016 a las 21:22

  • No debería tener ninguna razón para definir tal general swap plantilla en un espacio de nombres muy específico que contiene solo tipos específicos. Simplemente defina una no plantilla swap sobrecarga para foo::bar. Deje el intercambio general a std::swapy solo proporcionan sobrecargas específicas.

    – PlantillaRex

    25/04/2016 a las 21:23


  • @PlantillaRex foo es una capa de indirección que usamos sobre la biblioteca estándar. Muchas de las plataformas que admitimos tienen bibliotecas estándar incompletas o con errores, y en esas plataformas haremos cosas como direct foo::shared_ptr a boost::shared_ptr en vez de std::shared_ptr. Una de nuestras plataformas tiene una vieja std::swap() que copia en lugar de mover, por lo que proporcionamos un reemplazo en foo.

    –Tavian Barnes

    25/04/2016 a las 21:32

  • @TavianBarnes Actualicé mi respuesta en respuesta a sus dos comentarios.

    – usuario6253369

    25/04/2016 a las 21:45

avatar de usuario
mojar

  • unique_ptr<T> requiere T* ser una NullablePointer [unique.ptr]p3
  • NullablePointer requiere valores de T* ser – estar Swappable [nullablepointer.requirements]p1
  • Swappable esencialmente requiere using std::swap; swap(x, y); para seleccionar una sobrecarga para x, y siendo lvalues ​​de tipo T* [swappable.requirements]p3

En el último paso, su tipo foo::bar produce una ambigüedad y por lo tanto viola los requisitos de unique_ptr. La implementación de libstdc++ es conforme, aunque diría que esto es bastante sorprendente.


La redacción es, por supuesto, un poco más complicada, porque es genérica.

[unique.ptr]p3

Si el tipo remove_reference_t<D>::pointer existe, entonces unique_ptr<T, D>::pointer será un sinónimo de
remove_reference_t<D>::pointer. De lo contrario unique_ptr<T,
D>::pointer
será un sinónimo de T*. El tipo unique_ptr<T,
D>::pointer
cumplirá los requisitos de NullablePointer.

(énfasis mío)

[nullablepointer.requirements]p1

A NullablePointer type es un tipo similar a un puntero que admite valores nulos. Un tipo P cumple con los requisitos de NullablePointer si:

  • […]
  • valores de tipo P son intercambiables (17.6.3.2),
  • […]

[swappable.requirements]p2

Un objeto t es intercambiable con un objeto u si y solo si:

  • las expresiones swap(t, u) y swap(u, t) son válidos cuando se evalúan en el contexto descrito a continuación, y
  • […]

[swappable.requirements]p3

El contexto en el que swap(t, u) y swap(u, t) se evalúan garantizará que se seleccione una función no miembro binaria llamada “swap” a través de la resolución de sobrecarga en un conjunto de candidatos que incluye:

  • los dos swap plantillas de funciones definidas en <utility> y
  • el conjunto de búsqueda producido por la búsqueda dependiente de argumentos.

Tenga en cuenta que para un tipo de puntero T*para fines de ADL, los espacios de nombres y clases asociados se derivan del tipo T. Por eso, foo::bar* posee foo como un espacio de nombres asociado. AVD para swap(x, y) donde ya sea x o y es un foo::bar* encontrará por lo tanto foo::swap.

  • Puedes encontrar un desglose más simple aquí para los que no hablan abogado. De todos modos, aunque libstdc++ ciertamente sigue las reglas aquí, no parece haber nada que requiera que use el intercambio de ADL dentro del reinicio, considerando que el intercambio de borradores de libcxx es funcionalmente equivalente. Atribuiré esto a la calidad de la implementación.

    – usuario6253369

    25/04/2016 a las 21:20

  • @ user6253369 es QoI por parte del OP aquí. Simplemente no proporcione plantillas generales que dupliquen los puntos de personalización de las plantillas STL y, especialmente, no proporcione sus propias clases a dichas plantillas STL cuando lo haga.

    – PlantillaRex

    25/04/2016 a las 21:27

  • @ user6253369 Para punteros elegantes que realmente definieron intercambios personalizados, usarlos puede ser más eficiente.

    – CT

    25/04/2016 a las 21:52

avatar de usuario
usuario6253369

El problema es la implementación de libstdc++ de unique_ptr. Esto es de su rama 4.9.2:

https://gcc.gnu.org/onlinedocs/gcc-4.9.2/libstdc++/api/a01298_source.html#l00339

  338       void
  339       reset(pointer __p = pointer()) noexcept
  340       {
  341     using std::swap;
  342     swap(std::get<0>(_M_t), __p);
  343     if (__p != pointer())
  344       get_deleter()(__p);
  345       }

Como puede ver, hay una llamada de intercambio no calificada. Ahora veamos la implementación de libcxx (libc++):

https://git.io/vKzhF

_LIBCPP_INLINE_VISIBILITY void reset(pointer __p = pointer()) _NOEXCEPT
{
    pointer __tmp = __ptr_.first();
    __ptr_.first() = __p;
    if (__tmp)
        __ptr_.second()(__tmp);
}

_LIBCPP_INLINE_VISIBILITY void swap(unique_ptr& __u) _NOEXCEPT
    {__ptr_.swap(__u.__ptr_);}

no llaman swap en el interior reset ni utilizan una llamada de intercambio no calificada.


La respuesta de Dyp proporciona un desglose bastante sólido de por qué libstdc++ es conforme, pero también por qué su código se romperá cada vez que swap se requiere que sea llamado por la biblioteca estándar. Para citar a TemplateRex:

No debería tener ninguna razón para definir tal general swap plantilla en un espacio de nombres muy específico que contiene solo tipos específicos. Simplemente defina una no plantilla swap sobrecarga para foo::bar. Deje el intercambio general a std::swapy solo proporcionan sobrecargas específicas. fuente

Como ejemplo, esto no se compilará:

std::vector<foo::bar> v;
std::vector<foo::bar>().swap(v);

Si está apuntando a una plataforma con una biblioteca estándar antigua/GCC (como CentOS), le recomendaría usar Boost en lugar de reinventar la rueda para evitar trampas como esta.

  • “Recomendaría usar Boost en lugar de reinventar la rueda” ¡De acuerdo! Hacemos esto para muchas cosas en foo. Desafortunadamente boost::swap() no hace las asignaciones de movimiento, sino que simplemente delega a std::swap.

    –Tavian Barnes

    25/04/2016 a las 21:55

  • @TavianBarnes Eso puede ser cierto, pero creo que el equivalente de Boost de unique_ptr usa la semántica de movimiento para el intercambio, por ejemplo.

    – usuario6253369

    25/04/2016 a las 22:05

  • Correcto, pero a menudo quiero hacer swap(a, b); dónde a y b son de un tipo no copiable. Es un dolor declarar una sobrecarga para cada tipo de solo movimiento que defino, por lo que necesito un swap() implementación que se mueve.

    –Tavian Barnes

    25 de abril de 2016 a las 22:12

  • @TavianBarnes No suelo dar consejos turbios, pero ¿ha considerado reemplazar el intercambio en el nivel de biblioteca compartida? Creo que eso es común en los sistemas integrados.

    – usuario6253369

    25 de abril de 2016 a las 22:18

  • @user6253369 Actualicé su enlace a GitHub directamente a la confirmación y lo ejecuté a través del acortador de enlaces de GitHub. Siéntase libre de revertir la edición si son las partes incorrectas.

    – Por quémarrh

    16 de julio de 2016 a las 17:23

Esta técnica se puede utilizar para evitar foo::swap() ser encontrado por ADL:

namespace foo
{
    namespace adl_barrier
    {
        template <typename T>
        void swap(T& a, T& b)
        {
            T tmp = std::move(a);
            a = std::move(b);
            b = std::move(tmp);
        }
    }

    using namespace adl_barrier;
}

Así es como Boost.Range es independiente begin()/end() se definen las funciones. Intenté algo similar antes de hacer la pregunta, pero no using adl_barrier::swap; en cambio, lo que no funciona.

En cuanto a si el fragmento en la pregunta debería funcionar tal cual, no estoy seguro. Una complicación que puedo ver es que unique_ptr puede tener personalizado pointer tipos de la Deleterque debe ser intercambiado con el habitual using std::swap; swap(a, b); modismo. Ese idioma está claramente roto para foo::bar* en la pregunta

  • Si es “totalmente general”, ¿por qué no simplemente importa std::swap dentro foo con una declaración de uso?

    – mojar

    25/04/2016 a las 21:20

  • @dyp foo::swap() fue escrito porque una de nuestras plataformas tiene una biblioteca estándar con un deficiente std::swap() (hace copias en lugar de movimientos). probablemente lo haré using std::swap; excepto en esa plataforma.

    –Tavian Barnes

    25/04/2016 a las 21:22

  • por cierto, creo que es mejor proteger su tipo ADL (es decir, foo::bar) en lugar de sus funciones (es decir, foo::swap), ya que esos son los que causan el dolor, y el número de funciones en namespace foo es abierto (alguien podría agregar foo::begin y foo::end algún día y luego tienes nuevos problemas, ADL-protegido foo::bar protege contra eso).

    – PlantillaRex

    25/04/2016 a las 21:44


¿Ha sido útil esta solución?