search

crossbario / autobahn-cpp

WAMP for C++ in Boost/Asio
https://crossbar.io/autobahn
Boost Software License 1.0
251 stars 104 forks source link

Fix -Wextra C++ warnings #209

Closed robinlinden closed 4 years ago

robinlinden commented 4 years ago

The only odd one here is this, I suppose:

autobahn_cpp-src/autobahn/wamp_session.ipp:753:51: error: implicitly-declared 'autobahn::wamp_challenge& autobahn::wamp_challenge::operator=(const autobahn::wamp_challenge&)' is deprecated [-Werror=deprecated-copy]
  753 |         challenge_object = wamp_challenge("ticket");

The other two are just not using arguments to functions or trying in the function declaration to const things returned by value.

oberstet commented 4 years ago

thanks for attention to and fixing these details! yeah, the only odd one is: why is it that copy-constructors are deprecated? seems strange. but anyways, this = default is the trick to make that warning go silent? as in

wamp_challenge & operator=( const wamp_challenge & ) = default;

what does that do? does that work on older compilers (c++11)?

robinlinden commented 4 years ago

It's just the implicitly generated ones that are deprecated (and only if you have a user-declared copy-constructor or a destructor)! If you specify wamp_challenge &operator=(const wamp_challenge &) = default; you explicitly ask the compiler to generate a default one. You use ctor = default in other places in the code, so it's nothing new in your code base. It's supported in C++11. :)

I guess the reasoning behind it is that if you need a destructor, copy-constructor, or copy-assignment, then you're probably messing around with special resources and need all 3.

The generation of the implicitly-defined copy assignment operator is deprecated if T has a user-declared destructor or user-declared copy constructor. | (since C++11)

And some references: https://en.cppreference.com/w/cpp/language/copy_assignment#Implicitly-defined_copy_assignment_operator https://en.cppreference.com/w/cpp/language/rule_of_three https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58407

robinlinden commented 4 years ago

At a second look at this, it would probably be fine to just delete the user-defined copy-constructor. Don't merge this yet and I'll give it a try later. :)

robinlinden commented 4 years ago

Updated with a better fix for the deprecated-copy warning. It was possible to just delete the unnecessary copy-constructor and let the compiler generate all of it. :D Thanks for asking about it! (I would've missed that if you hadn't.)

oberstet commented 4 years ago

. It was possible to just delete the unnecessary copy-constructor and let the compiler generate all of it.

even better =)