[Gelöst] Spezialisierung std::swap



  • Hi zusammen,

    wir haben grad auf einen neuen Compiler gewechselt (Embarcadero, basierend auf clang 5.0) und der neue Compiler meckert, ich weiß nicht, warum:

    #ifndef MyClassH
    #define MyClassH
    
    #include <algorithm>
    
    namespace A {
    namespace B {
    namespace C {
    
    class MyClass
    {
       double Value1_ = 0.0;
       double Value2_ = 0.0;
    public:
       void swap( MyClass& other ) noexcept
       {
          std::swap( Value1_, other.Value1_ );
          std::swap( Value2_, other.Value2_ );
       }
    };
    
    } // namespace C
    } // namespace B
    } // namespace A
    
    namespace std
    {
       template<> void swap( A::B::C::MyClass& lhs, A::B::C::MyClass& rhs ) noexcept { lhs.swap( rhs ); }
    }
    #endif
    

    Fehlermeldung des Compilers:

    [bcc32c Fehler] MyClass.h(28): conflicting types for 'swap'
    [bcc32c Hinweis] MyClass.h(28): previous declaration is here
    

    Hab schon sämtliche Kombinationen mit constexpr, static und inline durch, die Fehlermeldung bleibt. Erst wenn ich die Funktion überlade (template<> entferne) läuft's durch, aber das ist dann illegal, weil man im namespace std nur spezialisieren darf. Die ursprüngliche Version wird auf Ideone anstandslos übersetzt.

    Edit: Frage ergänzt 😉
    Wie lautet die korrekte Signatur für die Spezialisierung von std::swap? Oder liegt der Compiler falsch?


  • Mod

    Ich bin gerade schwer verwirrt. Wo ist der Unterschied zwischen hier und der funktionierenden Version auf ideone? Das sieht für mich beides exakt gleich aus. Oder ist das Problem, dass diese Version nur auf Ideone funktioniert, aber bei dir nicht?

    // Hier
    template<> void swap( A::B::C::MyClass& lhs, A::B::C::MyClass& rhs ) noexcept { lhs.swap( rhs ); }
    // Ideone
    template<> void swap( A::B::C::MyClass& lhs, A::B::C::MyClass& rhs ) noexcept { lhs.swap( rhs ); }
    

    Wie lautet die vollständige Fehlermeldung? Er sagt ja, was er von swap erwarten würde.



  • Ja, genau, das verwirrt mich ja. Auf Ideone kompiliert's ohne Probleme, in meiner IDE weigert sich der Compiler mit der entsprechenden Fehlermeldung. Auf wandbox und C++ Shell kompiliert's auch, nur halt in meiner IDE nicht.


  • Mod

    Andere Frage: Wozu überhaupt die Spezialisierung in std? Wäre folgendes nicht viel besser?

    
    class MyClass
    {
       double Value1_ = 0.0;
       double Value2_ = 0.0;
    public:
       friend void swap( MyClass& self, MyClass& other ) noexcept
       {
          std::swap( self.Value1_, other.Value1_ );
          std::swap( self.Value2_, other.Value2_ );
       }
    };
    

    Das wäre viel freundlicher bezüglich Überladungsauflösung, insofern als dass es halt an Überladungsauflösung teilnimmt, die Templatespezialisierung hingegen nicht.

    PS: Wobei diese Variante nicht mit deinem speziellen Ideone-Beispiel funktionieren würde, da du dort explizit std::swap aufrufst, wohingegen der empfohlene Aufruf using std::swap; swap( mc1, mc2 ); wäre. Siehe https://en.cppreference.com/w/cpp/named_req/Swappable



  • Bin da offen, ich meine nur iwann mal gelesen zu haben, dass man die entsprechende swap Funktion im std-namespace spezialisieren soll. Da die Klasse selbst schon eine öffentliche swap-Funktion mitbringt könnte ich sie auch ohne friend implementieren. Die Frage ist jetzt nur: Wo implementiere ich die swap-Funktion? Als Spezialisierung im std-namespace oder im Namespace, in der die Klasse liegt?


  • Mod

    Die sollte normalerweise im gleichen Namespace wie die Klasse liegen, damit sie über ADL gefunden werden. Was ja automatisch der Fall ist, wenn man sie inline als Friend definiert, weswegen ich das auch bewusst so gezeigt habe.



  • @DocShoe sagte in Spezialisierung std::swap:

    Hi,

    mach mal:

    void swap( MyClass& other ) noexcept
       {
          using std::swap;
          std::swap( Value1_, other.Value1_ );
          std::swap( Value2_, other.Value2_ );
       }
    

    (Meyers idiom #25 für eine detailierte Erklärung)

    Edit: Bin gerade etwas verwirrt, wer bei dir was aufruft. Der Meyers-Tipp für die korrekte Implementierung kann aber stehen bleiben.



  • Erstmal vielen Dank für eure Hilfe.
    Es scheint allerdings komplizierter zu sein, als befürchtet. Ich habe mir grad mal den Meyers (Addison Wesley: Effektiv C++ programmieren, 3. Auflage, deutsch) geschnappt und nachgelesen, da steht auf Seite 137:
    [Scott Meyers]

    1. Bieten Sie eine Elementfunktion swap an, wenn std::swap für Ihren Datentyp zu ineffizient sein sollte. Stellen Sie sicher, dass Ihr swap keine Ausnahme auslöst.
    2. Wenn Sie eine Elementfunktion swap anbieten, liefern sie auch eine Nichtelementfunktion swap, die die Elementfunktion aufruft. Für Klassen (nicht für Klassen-Templates) spezialisieren Sie auch std::swap.

    Ich habe beide Punkt befolgt, daher auch die Spezialisierung in Zeile 28 in meinem Eingangpost. Ich möchte eine swap Funktion anbieten, die auch von Algorithmen der STL aufgerufen wird, da scheint mir die Spezialisierung im std-namespace notwendig. Und wenn ich den Meyers richtig verstanden habe, sollte man 3 swap Funktionen anbieten: eine Memberfunktion, eine freie Funktion im gleichen namespace wie die Klasse und eine Spezialisierung im std-Namespace. Die freien Funktionen sollten dann alle die Memberfunktion benutzen (Seite 136f).

    @Jockelx
    So wie ich das verstehe ist das der Anwendungsfall für swap, aber nicht die Implementierung. Per using-Direktive wird die std::swap Funktion bekannt gemacht, die als Fallback benutzt wird, wenn es keine Überladung/Spezialisierung für den zu tauschenden Typ gibt. In meinem Fall möchte ich aber erstmal nichts tauschen, sondern nur eine swap Funktion anbieten, die benutzt werden soll, sowohl von normalem Client Code als auch der STL. Das Beispiel im Buch sieht so aus und benutzt die swap-Funktion:

    template<typename T>
    void doSomething( T& obj1, T& obj 2 )
    {
       using std::swap;
       swap( obj1, obj2 );
    }
    

    Da T hier unbekannt ist kann auch nicht sichergestellt werden, dass es eine dedizierte swap Funktion gibt. In meinem Fall gibt es aber ganz sicher eine, weil ich sie ja implementieren möchte.



  • @DocShoe sagte in Spezialisierung std::swap:

    So wie ich das verstehe ist das der Anwendungsfall für swap, aber nicht die Implementierung

    Hi,

    ja, der erste Teil dreht sich darum, wie man das implementiert, aber da macht er es ja genauso wie du.
    Sorry, da hab ich nicht richtig geguckt und hatte nur was mit "da muss std::swap in den namespace mit rein", was mich getriggert hat.
    Was anderes: Gibt es einen Unterschied, wenn du den Typ in der Spezialisierung nochmal explizit angibst?

    namespace std
    {
       template<> void swap<A::B::C::MyClass>( A::B::C::MyClass& lhs, A::B::C::MyClass& rhs ) noexcept { lhs.swap( rhs ); }
    }
    


  • @DocShoe Doofe Frage: MyClass ist nur Minimalbeispiel, oder? Ich gehe davon aus, dass es für die tatsächliche Klasse einen guten Grund gibt, swap zu implementieren (z.B. weil Move/Copy-Constructor/Assignment nicht zur Verfügung stehen, du diese über swap implementieren willst oder aus Performance-Gründen)? Ich denke mal ja.

    Ansonsten bin ich auch überrascht wegen der Fehlermeldung. Ich hätte erwartet, dass man das exakt so implementiert und kann auch auf Godbolt keinen Compiler finden, der bei deinem Beispiel meckert.

    swap ist allerdings auch eine Funktionalität der C++Standardbibliothek und soweit ich weiss verwendet der Embarcadero Compiler - im Gegensatz zum LLVM clang 5 hier eine implementierung von Dinkumware. Ich würde mal tippen, dass dort die Ursache dieser Fehlermeldung zu finden ist. Zur Dinkumware-Implementierung kann ich leider nicht viele frei verfügbare Informationen finden - eventuell solltest du dir mal deren <algorithm>-Header ansehen und herausfinden, wie dort std::swap umgesetzt wurde.


  • Mod

    Die Idee ist ja, wenn die Standardbibliothek (und andere Libraries natürlich auch), es so machen, wie Meyers es empfiehlt (und das sollten sie, denn darauf beruht das Swappable Konzept in C++17/C++20), dann brauchst du keine Spezialisierung in std, sondern eine Überladung im eigenen Namespace ist ausreichend und besser. Und vor allem entfallen die ganzen Schwierigkeiten vonwegen der Spezialisierung in std, das ist quasi die Hauptmotivation hinter diesem Konzept. Streng genommen darf man ab C++20 swap gar nicht mehr in std spezialisieren, vielleicht ist es das was deinen Compiler stört.

    C++20 Standard 16.4.5.2.1, 2 sagte:

    Unless explicitly prohibited, a program may add a template specialization for any standard library class template to namespace std provided that (a) the added declaration depends on at least one program-defined type and (b) the specialization meets the standard library requirements for the original template

    Explizit so geändert, dass da nur noch Klassenspezialisierungen zugelassen sind, keine Funktionstemplates.



  • @SeppJ sagte in Spezialisierung std::swap:

    Die Idee ist ja, wenn die Standardbibliothek (und andere Libraries natürlich auch), es so machen, wie Meyers es empfiehlt [...]

    Bin auf einen uralten Thread gestossen, der interessanterweise die Dinkumware STL zu diesem Thema erwähnt:

    @Werner-Salomon sagte in swap, abs & Co im template mit oder ohne std:::

    Wirft man jetzt aber einen Blick in die Implementierung der (Dinkumware-)STL so findet man dort das explicite std::swap vor, was zwar i.A. funktionieren wird, aber eben bei selbst geschriebener swap-Funktion, die sich nicht im namespace std befindet (darf sie das?, soll sie das?) nicht das gewünschte Ergebnis liefert.

    Ich hoffe mal, dass sich das in den letzten 16 Jahren geändert hat, sonst bräuchte man soweit ich beurteilen kann tatsächlich die std-Spezialisierung. Noch ein Grund mehr, sich den Code der Standardbibliothek mal anzusehen bzw. Tests zu schreiben, die feststellen, ob das eigene swap tatsächlich aufgerufen wird.


  • Mod

    @DocShoe Kannst Du die Implementierung von Deiner stdlib einsehen und die Funktionsprototypen posten? Koennte ja irgendwas albernes sein, wie ein fehlendes noexcept oder so.



  • @Finnegan
    Nein, das ist nur ein Minimalbeispiel, die echte Klasse hat noch einen std::vector<double>, bei dem ich unnötige Kopien gern vermeiden würde.

    Eigentlich ist es noch verrückter:

    Ich habe ein paar Klassen, für die ich die swap Funktion anbieten möchte. Am Ende der Header Datei habe ich einen Block:

    namespace A {
    namespace B {
    namespace C {
    
    class MyClassA
    {
    public:
       void swap( MyClassA& other ) noexcept
       {
       }
    };
    
    class MyClassB
    {
    public:
       void swap( MyClassB& other ) noexcept
       {
       }
    };
    
    ...
    // 3 weitere Klassen
    ...
    
    } // namespace C
    } // namespace B
    } // namespace A
    
    namespace std
    {
       template<> void swap( A::B::C::MyClassA& lhs, A::B::C::MyClassA& rhs ) noexcept { lhs.swap( rhs ); }
       template<> void swap( A::B::C::MyClassB& lhs, A::B::C::MyClassB& rhs ) noexcept { lhs.swap( rhs ); }
       template<> void swap( A::B::C::MyClassC& lhs, A::B::C::MyClassC& rhs ) noexcept { lhs.swap( rhs ); }
       template<> void swap( A::B::C::MyClassD& lhs, A::B::C::MyClassD& rhs ) noexcept { lhs.swap( rhs ); }
       template<> void swap( A::B::C::MyClassE& lhs, A::B::C::MyClassE& rhs ) noexcept { lhs.swap( rhs ); }
    }
    

    Das sind alles Klassen, die keine Abhängigkeiten, d.h. keine Klasse erbt von irgendwas. Der Compiler bemängelt nur die Zeilen 5 & 6, alle anderen gehen.

    Die Dinkumware-Implementation der swap-Funktion sieht so aus:

    template<class _Ty, class = enable_if_t<is_move_constructible<_Ty>::value && is_move_assignable<_Ty>::value, void> > inline
    void swap(_Ty& _Left, _Ty& _Right) _NOEXCEPT_OP(is_nothrow_move_constructible<_Ty>::value && is_nothrow_move_assignable<_Ty>::value)
    {	
       // exchange values stored at _Left and _Right
       _Ty _Tmp(_Move(_Left));
       _Left = _Move(_Right);
       _Right = _Move(_Tmp);
    }
    


  • Langsam kommt Licht in's Dunkel. Die beiden Klassen, für die der Compiler Fehler anzeigt, haben einen expliziten Konstruktor, alle anderen nicht. Die einzige Erklärung, die ich da habe, ist dass mindestens eins der Prädikate is_move_constructible, is_move_assignable, is_nothrow_move_constructible oder is_nothrow_move_assignable nicht erfüllt ist. Ich habe eine Klasse dann um die passenden Konstruktoren/Assignment Operatoren erweitert:

    class MyClassA
    {
    public:
       MyClassA() = default;
       explicit MyClass( std::size_t window_size );
    
       MyClassA( MyClassA const& other ) = default;
       MyClassA( MyClassA && other ) = default;
    
       MyClassA& operator=( MyClassA const& other ) = default;
       MyClassA& operator=( MyClassA && other ) = default;
    };
    

    Das Problem bleibt, aber das ist mal ein Ansatz.

    So, weiter gegraben und das hier festgestellt: Die beiden Klassen benutzen intern eine Klasse RingBuffer<double>, die selbst die Rule of Five implementiert. Für RingBuffer<T> darf keine swap-Spezialisierung in der Form

    namespace std
    {
       template<typename T>
       void swap( RingBuffer<T>& lhs, RingBuffer<T>& rhs ) noexcept { /*... */ }
    }
    

    implementiert werden, weil man das nicht tun soll (versuche grade noch rauszufinden, warum man das nicht tun soll).

    Edit:
    Erklärung: Man darf Templates im std-Namespace spezialisieren, aber keine neuen Funktionen hinzufügen. Weil das obige swap keine Spezialisierung ist, sondern eine neue swap-Funktion für RingBuffer-templates ist, ist das illegal. Ich dürfte std::swap nur für konkrete RingBuffer-Typen spezialisieren:

    namespace std
    {
       template<>
       void swap( RingBuffer<double>& lhs, RingBuffer<double>& rhs ) noexcept { /* ...  */ }
    }
    


  • Soooo... Problem gefunden und gelöst. Das Problem war die Klasse RingBuffer, die zwar die Rule of Five implementiert, aber eben nicht richtig, weil der Move-Constructor und Move-Assignment nicht noexcept waren. Ich habe das ergänzt und jetzt kompiliert's. Werd' jetzt mal alle Move-Constructor und Move-Assignment prüfen, ob sie noexcept sind.
    Vielen Dank nochmal.


  • Mod

    Das heißt, der Code, wie im ersten Beitrag gezeigt, hätte bei dir compiliert? Und erst bei einem nicht gezeigten Ringbuffermember würde der Fehler auftreten? Ich sollte hier irgendwas über die Erstellung von Minimalbeispielen schreiben, von dem ich eigentlich denken würde, das du es wüsstest.



  • Vermutlich.
    Das Minimalbeispiel wäre wahrscheinlich ziemlich groß geworden, weil der RingBuffer selbst wieder einige Abhängigkeiten hat.


  • Mod

    @DocShoe sagte in [Gelöst] Spezialisierung std::swap:

    Vermutlich.
    Das Minimalbeispiel wäre wahrscheinlich ziemlich groß geworden, weil der RingBuffer selbst wieder einige Abhängigkeiten hat.

    Der Punkt ist eher, dass du bei der Erstellung des Minimalbeispiels stets hättest ausprobieren müssen, ob der Fehler noch auftritt. Dann wäre aufgefallen, dass der Fehler nicht mehr auftritt, wenn man den Ringbuffer entfernt. Dann wäre die Fehlersuche viel gezielter verlaufen, anstatt hier vermeintliche Compilerfehler zu jagen.



  • @DocShoe aber überprüfen ob in einem geposteten Minimalbeispiel der Fehler auch wirklich Auftritt würde einiges rätseln ersparen 😉

    Edit: @SeppJ war schneller


Anmelden zum Antworten