WString: mark move ctor as noexcept

[mcspr]

ref.

• https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-move-noexcept
• https://rules.sonarsource.com/cpp/RSPEC-5018?search=noexecept
• https://clang.llvm.org/extra/clang-tidy/checks/performance-noexcept-move-constructor.html

Move constructors of all the types used with STL containers, for
example, need to be declared noexcept. Otherwise STL will choose copy
constructors instead. The same is valid for move assignment operations.

With a basic example based on the master

diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index 46aad5d5..1c93d8f2 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -225,6 +225,7 @@ String & String::copy(const __FlashStringHelper *pstr, unsigned int length) { #ifdef __GXX_EXPERIMENTAL_CXX0X__ void String::move(String &rhs) { + Serial.printf("moved! %p -> %p\n", &rhs, this); if (buffer()) { if (capacity() >= rhs.len()) { memmove_P(wbuffer(), rhs.buffer(), rhs.length() + 1);

#include <Arduino.h> #include <vector> void setup() { Serial.begin(115200); std::vector<String> objects; for (int num = 0; num < 10; ++num) { objects.emplace_back(num); } } void loop() { delay(1000); ESP.restart(); }

Since the vector is not using .reserve(...), we need to realloc + move existing vector contents every push when vector needs more capacity. Without the patch, we will never see the moved! string from the above.

Core does use std::vector<String> in a couple of places as well, it is not just possible user code:

git grep 'std::vector<Str' libraries/ESP8266WebServer/src/Uri.h: virtual bool canHandle(const String &requestUri, __attribute__((unused)) std::vector<String> &pathArgs) { libraries/ESP8266WebServer/src/detail/RequestHandler.h: std::vector<String> pathArgs; libraries/ESP8266WebServer/src/uri/UriBraces.h: bool canHandle(const String &requestUri, std::vector<String> &pathArgs) override final { libraries/ESP8266WebServer/src/uri/UriGlob.h: bool canHandle(const String &requestUri, __attribute__((unused)) std::vector<String> &pathArgs) override final { libraries/ESP8266WebServer/src/uri/UriRegex.h: bool canHandle(const String &requestUri, std::vector<String> &pathArgs) override final { 

(But, I am not sure what is the implication for the overall Arduino API stuff. Does it need the patch, too, if this is valid?)

[devyte]

One of things that is pending is to mark methods as noexcept across the entire code base. This is a good start, and for very good reason.

[devyte]

Also, if something breaks, now is the time for it, the sooner the better. There is time to detect and figure out such cases.

[mcspr]

One of things that is pending is to mark methods as noexcept across the entire code base. This is a good start, and for very good reason.

Any hints as to where to look / any specific cases besides T(T&&) and operator=(T&&)
?
Only WiFiClient caught my eye recently. And there is also a logic bug where move ctor is implicitly created, std::is_move_constructible<WiFiClient> is true, causing old object destructor to delete kind-of 'moved' ClientContext pointer.