From c6c224a78715d5e2c532fe318325fbca8e70e701 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 9 Dec 2022 20:00:08 +0300 Subject: Add noexcept to move constructors and move assignment operators --- libbutl/b.cxx | 2 ++ libbutl/diagnostics.hxx | 2 +- libbutl/filesystem.hxx | 4 ++-- libbutl/filesystem.ixx | 4 ++-- libbutl/mingw-thread.hxx | 2 +- libbutl/move-only-function.hxx | 5 ++++- libbutl/optional.hxx | 33 +++++++++++++++++++++++++++------ libbutl/optional.ixx | 5 +++++ libbutl/path.hxx | 8 ++++---- libbutl/path.ixx | 8 ++++---- libbutl/process.hxx | 16 ++++++++-------- libbutl/process.ixx | 16 ++++++++-------- libbutl/small-forward-list.hxx | 9 ++++++++- libbutl/small-list.hxx | 15 +++++++++++---- libbutl/small-vector.hxx | 33 +++++++++++++++++++++++++++++---- libbutl/utility.hxx | 4 ++-- libbutl/utility.ixx | 4 ++-- libbutl/uuid.hxx | 4 ++-- libbutl/uuid.ixx | 4 ++-- 19 files changed, 124 insertions(+), 54 deletions(-) diff --git a/libbutl/b.cxx b/libbutl/b.cxx index 3d78803..0b4472f 100644 --- a/libbutl/b.cxx +++ b/libbutl/b.cxx @@ -83,6 +83,8 @@ namespace butl string spec ("info("); + // Note that quoting is essential here. + // for (size_t i (0); i != projects.size(); ++i) { if (i != 0) diff --git a/libbutl/diagnostics.hxx b/libbutl/diagnostics.hxx index d805c20..c6db34b 100644 --- a/libbutl/diagnostics.hxx +++ b/libbutl/diagnostics.hxx @@ -166,7 +166,7 @@ namespace butl #endif empty_ (r.empty_), epilogue_ (r.epilogue_), - os (std::move (r.os)) + os (std::move (r.os)) // Note: can throw. { if (!empty_) { diff --git a/libbutl/filesystem.hxx b/libbutl/filesystem.hxx index fa5f30a..4cb74a6 100644 --- a/libbutl/filesystem.hxx +++ b/libbutl/filesystem.hxx @@ -215,8 +215,8 @@ namespace butl // Movable-only type. Move-assignment cancels the lhs object. // - auto_rm (auto_rm&&); - auto_rm& operator= (auto_rm&&); + auto_rm (auto_rm&&) noexcept; + auto_rm& operator= (auto_rm&&) noexcept; auto_rm (const auto_rm&) = delete; auto_rm& operator= (const auto_rm&) = delete; diff --git a/libbutl/filesystem.ixx b/libbutl/filesystem.ixx index 79607d7..b3f9224 100644 --- a/libbutl/filesystem.ixx +++ b/libbutl/filesystem.ixx @@ -73,7 +73,7 @@ namespace butl // template inline auto_rm

:: - auto_rm (auto_rm&& x) + auto_rm (auto_rm&& x) noexcept : path (std::move (x.path)), active (x.active) { x.active = false; @@ -81,7 +81,7 @@ namespace butl template inline auto_rm

& auto_rm

:: - operator= (auto_rm&& x) + operator= (auto_rm&& x) noexcept { if (this != &x) { diff --git a/libbutl/mingw-thread.hxx b/libbutl/mingw-thread.hxx index b308dde..66f98aa 100644 --- a/libbutl/mingw-thread.hxx +++ b/libbutl/mingw-thread.hxx @@ -154,7 +154,7 @@ namespace mingw_stdthread native_handle_type native_handle() const {return mHandle;} thread(): mHandle(kInvalidHandle), mThreadId(){} - thread(thread&& other) + thread(thread&& other) noexcept :mHandle(other.mHandle), mThreadId(other.mThreadId) { other.mHandle = kInvalidHandle; diff --git a/libbutl/move-only-function.hxx b/libbutl/move-only-function.hxx index 846ef25..e5cfe51 100644 --- a/libbutl/move-only-function.hxx +++ b/libbutl/move-only-function.hxx @@ -124,7 +124,10 @@ namespace butl return f (std::forward (args)...); } - wrapper (wrapper&& w): f (std::move (w.f)) {} + wrapper (wrapper&& w) + noexcept (std::is_nothrow_move_constructible::value) + : f (std::move (w.f)) {} + wrapper& operator= (wrapper&&) = delete; // Shouldn't be needed. ~wrapper () {f.~F ();} diff --git a/libbutl/optional.hxx b/libbutl/optional.hxx index 7d66ac5..f22189b 100644 --- a/libbutl/optional.hxx +++ b/libbutl/optional.hxx @@ -108,10 +108,16 @@ namespace butl #if (!defined(_MSC_VER) || _MSC_VER > 1900) && \ (!defined(__GNUC__) || __GNUC__ > 4 || defined(__clang__)) constexpr optional_data (const optional_data& o): v_ (o.v_) {if (v_) new (&d_) T (o.d_);} - constexpr optional_data (optional_data&& o): v_ (o.v_) {if (v_) new (&d_) T (std::move (o.d_));} + + constexpr optional_data (optional_data&& o) + noexcept (std::is_nothrow_move_constructible::value) + : v_ (o.v_) {if (v_) new (&d_) T (std::move (o.d_));} #else optional_data (const optional_data& o): v_ (o.v_) {if (v_) new (&d_) T (o.d_);} - optional_data (optional_data&& o): v_ (o.v_) {if (v_) new (&d_) T (std::move (o.d_));} + + optional_data (optional_data&& o) + noexcept (std::is_nothrow_move_constructible::value) + : v_ (o.v_) {if (v_) new (&d_) T (std::move (o.d_));} #endif optional_data& operator= (nullopt_t); @@ -119,7 +125,11 @@ namespace butl optional_data& operator= (T&&); optional_data& operator= (const optional_data&); - optional_data& operator= (optional_data&&); + + optional_data& operator= (optional_data&&) + noexcept (std::is_nothrow_move_constructible::value && + std::is_nothrow_move_assignable::value && + std::is_nothrow_destructible::value); ~optional_data (); }; @@ -151,10 +161,16 @@ namespace butl #if (!defined(_MSC_VER) || _MSC_VER > 1900) && \ (!defined(__GNUC__) || __GNUC__ > 4 || defined(__clang__)) constexpr optional_data (const optional_data& o): v_ (o.v_) {if (v_) new (&d_) T (o.d_);} - constexpr optional_data (optional_data&& o): v_ (o.v_) {if (v_) new (&d_) T (std::move (o.d_));} + + constexpr optional_data (optional_data&& o) + noexcept (std::is_nothrow_move_constructible::value) + : v_ (o.v_) {if (v_) new (&d_) T (std::move (o.d_));} #else optional_data (const optional_data& o): v_ (o.v_) {if (v_) new (&d_) T (o.d_);} - optional_data (optional_data&& o): v_ (o.v_) {if (v_) new (&d_) T (std::move (o.d_));} + + optional_data (optional_data&& o) + noexcept (std::is_nothrow_move_constructible::value) + : v_ (o.v_) {if (v_) new (&d_) T (std::move (o.d_));} #endif optional_data& operator= (nullopt_t); @@ -162,7 +178,12 @@ namespace butl optional_data& operator= (T&&); optional_data& operator= (const optional_data&); - optional_data& operator= (optional_data&&); + + // Note: it is trivially destructible and thus is no-throw destructible. + // + optional_data& operator= (optional_data&&) + noexcept (std::is_nothrow_move_constructible::value && + std::is_nothrow_move_assignable::value); }; template inline optional_data& optional_data:: operator= (optional_data&& o) + noexcept (std::is_nothrow_move_constructible::value && + std::is_nothrow_move_assignable::value && + std::is_nothrow_destructible::value) { if (o.v_) { @@ -171,6 +174,8 @@ namespace butl template inline optional_data& optional_data:: operator= (optional_data&& o) + noexcept (std::is_nothrow_move_constructible::value && + std::is_nothrow_move_assignable::value) { if (o.v_) { diff --git a/libbutl/path.hxx b/libbutl/path.hxx index 70c15d4..b10022a 100644 --- a/libbutl/path.hxx +++ b/libbutl/path.hxx @@ -1479,9 +1479,9 @@ namespace butl basic_path_name (): // Create empty/NULL path name. base (nullptr, &name) {} - basic_path_name (basic_path_name&&); + basic_path_name (basic_path_name&&) noexcept; basic_path_name (const basic_path_name&); - basic_path_name& operator= (basic_path_name&&); + basic_path_name& operator= (basic_path_name&&) noexcept; basic_path_name& operator= (const basic_path_name&); }; @@ -1508,9 +1508,9 @@ namespace butl basic_path_name_value (): base (&path) {} // Create empty/NULL path name. - basic_path_name_value (basic_path_name_value&&); + basic_path_name_value (basic_path_name_value&&) noexcept; basic_path_name_value (const basic_path_name_value&); - basic_path_name_value& operator= (basic_path_name_value&&); + basic_path_name_value& operator= (basic_path_name_value&&) noexcept; basic_path_name_value& operator= (const basic_path_name_value&); }; } diff --git a/libbutl/path.ixx b/libbutl/path.ixx index d2084f0..b2fdb6f 100644 --- a/libbutl/path.ixx +++ b/libbutl/path.ixx @@ -782,7 +782,7 @@ namespace butl // template inline basic_path_name

:: - basic_path_name (basic_path_name&& p) + basic_path_name (basic_path_name&& p) noexcept : basic_path_name (p.path, std::move (p.name)) { } @@ -796,7 +796,7 @@ namespace butl template inline basic_path_name

& basic_path_name

:: - operator= (basic_path_name&& p) + operator= (basic_path_name&& p) noexcept { if (this != &p) { @@ -824,7 +824,7 @@ namespace butl // template inline basic_path_name_value

:: - basic_path_name_value (basic_path_name_value&& p) + basic_path_name_value (basic_path_name_value&& p) noexcept : basic_path_name_value (std::move (p.path), std::move (p.name)) { } @@ -838,7 +838,7 @@ namespace butl template inline basic_path_name_value

& basic_path_name_value

:: - operator= (basic_path_name_value&& p) + operator= (basic_path_name_value&& p) noexcept { if (this != &p) { diff --git a/libbutl/process.hxx b/libbutl/process.hxx index dfade07..8648690 100644 --- a/libbutl/process.hxx +++ b/libbutl/process.hxx @@ -117,8 +117,8 @@ namespace butl // Moveable-only type. // - process_path (process_path&&); - process_path& operator= (process_path&&); + process_path (process_path&&) noexcept; + process_path& operator= (process_path&&) noexcept; process_path (const process_path&) = delete; process_path& operator= (const process_path&) = delete; @@ -352,8 +352,8 @@ namespace butl // Moveable-only type. // - pipe (pipe&&); - pipe& operator= (pipe&&); + pipe (pipe&&) noexcept; + pipe& operator= (pipe&&) noexcept; pipe (const pipe&) = delete; pipe& operator= (const pipe&) = delete; @@ -499,8 +499,8 @@ namespace butl // Moveable-only type. // - process (process&&); - process& operator= (process&&); + process (process&&) noexcept; + process& operator= (process&&) noexcept (false); // Note: calls wait(). process (const process&) = delete; process& operator= (const process&) = delete; @@ -749,8 +749,8 @@ namespace butl // Moveable-only type. // - process_env (process_env&&); - process_env& operator= (process_env&&); + process_env (process_env&&) noexcept; + process_env& operator= (process_env&&) noexcept; process_env (const process_env&) = delete; process_env& operator= (const process_env&) = delete; diff --git a/libbutl/process.ixx b/libbutl/process.ixx index 9753d22..e4db474 100644 --- a/libbutl/process.ixx +++ b/libbutl/process.ixx @@ -35,7 +35,7 @@ namespace butl args0_ (nullptr) {} inline process_path:: - process_path (process_path&& p) + process_path (process_path&& p) noexcept : effect (std::move (p.effect)), args0_ (p.args0_) { @@ -48,7 +48,7 @@ namespace butl } inline process_path& process_path:: - operator= (process_path&& p) + operator= (process_path&& p) noexcept { if (this != &p) { @@ -127,14 +127,14 @@ namespace butl // process::pipe // inline process::pipe:: - pipe (pipe&& p) + pipe (pipe&& p) noexcept : in (p.in), out (p.out), own_in (p.own_in), own_out (p.own_out) { p.in = p.out = -1; } inline process::pipe& process::pipe:: - operator= (pipe&& p) + operator= (pipe&& p) noexcept { if (this != &p) { @@ -411,7 +411,7 @@ namespace butl } inline process:: - process (process&& p) + process (process&& p) noexcept : handle (p.handle), exit (std::move (p.exit)), out_fd (std::move (p.out_fd)), @@ -422,7 +422,7 @@ namespace butl } inline process& process:: - operator= (process&& p) + operator= (process&& p) noexcept (false) { if (this != &p) { @@ -459,13 +459,13 @@ namespace butl // process_env // inline process_env:: - process_env (process_env&& e) + process_env (process_env&& e) noexcept { *this = std::move (e); } inline process_env& process_env:: - operator= (process_env&& e) + operator= (process_env&& e) noexcept { if (this != &e) { diff --git a/libbutl/small-forward-list.hxx b/libbutl/small-forward-list.hxx index 1278dc2..8d1cf68 100644 --- a/libbutl/small-forward-list.hxx +++ b/libbutl/small-forward-list.hxx @@ -5,6 +5,7 @@ #include // size_t #include // move() +#include // is_nothrow_move_constructible #include #include @@ -101,14 +102,20 @@ namespace butl return *this; } + // See small_vector for the move-constructor/assignment noexept + // expressions reasoning. + // small_forward_list (small_forward_list&& v) +#if !defined(_MSC_VER) || _MSC_VER > 1900 + noexcept (std::is_nothrow_move_constructible::value) +#endif : base_type (allocator_type (this)) { *this = std::move (v); // Delegate to operator=(&&). } small_forward_list& - operator= (small_forward_list&& v) + operator= (small_forward_list&& v) noexcept (false) { // VC14's implementation of operator=(&&) swaps pointers without regard // for allocator (fixed in 15). diff --git a/libbutl/small-list.hxx b/libbutl/small-list.hxx index aaeef22..7cb51fd 100644 --- a/libbutl/small-list.hxx +++ b/libbutl/small-list.hxx @@ -4,8 +4,9 @@ #pragma once #include -#include // size_t -#include // move() +#include // size_t +#include // move() +#include // is_nothrow_move_constructible #include @@ -103,14 +104,20 @@ namespace butl return *this; } + // See small_vector for the move-constructor/assignment noexept + // expressions reasoning. + // small_list (small_list&& v) +#if !defined(__GLIBCXX__) && (!defined(_MSC_VER) || _MSC_VER > 1900) + noexcept (std::is_nothrow_move_constructible::value) +#endif : base_type (allocator_type (this)) { *this = std::move (v); // Delegate to operator=(&&). } small_list& - operator= (small_list&& v) + operator= (small_list&& v) noexcept (false) { // libstdc++'s implementation prior to GCC 6 is broken (calls swap()). // Since there is no easy way to determine this library's version, for @@ -122,7 +129,7 @@ namespace butl #if defined(__GLIBCXX__) || (defined(_MSC_VER) && _MSC_VER <= 1900) this->clear (); for (T& x: v) - this->push_back (std::move (x)); + this->push_back (std::move (x)); // Note: can throw bad_alloc. v.clear (); #else // Note: propagate_on_container_move_assignment = false diff --git a/libbutl/small-vector.hxx b/libbutl/small-vector.hxx index 9e2a357..44a3ef5 100644 --- a/libbutl/small-vector.hxx +++ b/libbutl/small-vector.hxx @@ -4,8 +4,9 @@ #pragma once #include -#include // size_t -#include // move() +#include // size_t +#include // move() +#include // is_nothrow_move_constructible #include @@ -107,7 +108,25 @@ namespace butl return *this; } - small_vector (small_vector&& v) noexcept + // Note that while the move constructor is implemented via the move + // assignment it may not throw if the value type is no-throw move + // constructible. + // + // Specifically, if v.size() > N then allocators evaluate as equal and the + // buffer ownership is transferred. Otherwise, the allocators do not + // evaluate as equal and the individual elements are move-constructed in + // the preallocated buffer. + // + // Also note that this constructor ends up calling + // base_type::operator=(base_type&&) whose noexcept expression evaluates + // to false (propagate_on_container_move_assignment and is_always_equal + // are false for small_allocator; see std::vector documentation for + // details). We, however, assume that the noexcept expression we use here + // is strict enough for all "sane" std::vector implementations since + // small_allocator never throws directly. + // + small_vector (small_vector&& v) + noexcept (std::is_nothrow_move_constructible::value) : base_type (allocator_type (this)) { if (v.size () <= N) @@ -121,8 +140,14 @@ namespace butl v.clear (); } + // Note that when size() <= N and v.size() > N, then allocators of this + // and other containers do not evaluate as equal. Thus, the memory for the + // new elements is allocated on the heap and so std::bad_alloc can be + // thrown. @@ TODO: maybe we could re-implement this case in terms of + // swap()? + // small_vector& - operator= (small_vector&& v) + operator= (small_vector&& v) noexcept (false) { // VC's implementation of operator=(&&) (both 14 and 15) frees the // memory and then reallocated with capacity equal to v.size(). This is diff --git a/libbutl/utility.hxx b/libbutl/utility.hxx index e284fa6..2785bac 100644 --- a/libbutl/utility.hxx +++ b/libbutl/utility.hxx @@ -314,8 +314,8 @@ namespace butl // Move-to-empty-only type. // - auto_thread_env (auto_thread_env&&); - auto_thread_env& operator= (auto_thread_env&&); + auto_thread_env (auto_thread_env&&) noexcept; + auto_thread_env& operator= (auto_thread_env&&) noexcept; auto_thread_env (const auto_thread_env&) = delete; auto_thread_env& operator= (const auto_thread_env&) = delete; diff --git a/libbutl/utility.ixx b/libbutl/utility.ixx index 45bbf9d..0ce33a7 100644 --- a/libbutl/utility.ixx +++ b/libbutl/utility.ixx @@ -371,14 +371,14 @@ namespace butl } inline auto_thread_env:: - auto_thread_env (auto_thread_env&& x) + auto_thread_env (auto_thread_env&& x) noexcept : prev_env (std::move (x.prev_env)) { x.prev_env = nullopt; } inline auto_thread_env& auto_thread_env:: - operator= (auto_thread_env&& x) + operator= (auto_thread_env&& x) noexcept { if (this != &x) { diff --git a/libbutl/uuid.hxx b/libbutl/uuid.hxx index 2361640..47a814c 100644 --- a/libbutl/uuid.hxx +++ b/libbutl/uuid.hxx @@ -158,10 +158,10 @@ namespace butl void swap (uuid&); - uuid (uuid&&); + uuid (uuid&&) noexcept; uuid (const uuid&) = default; - uuid& operator= (uuid&&); + uuid& operator= (uuid&&) noexcept; uuid& operator= (const uuid&) = default; }; diff --git a/libbutl/uuid.ixx b/libbutl/uuid.ixx index 6744af7..6115be1 100644 --- a/libbutl/uuid.ixx +++ b/libbutl/uuid.ixx @@ -39,14 +39,14 @@ namespace butl } inline uuid:: - uuid (uuid&& u) + uuid (uuid&& u) noexcept : uuid () // nil { swap (u); } inline uuid& uuid:: - operator= (uuid&& u) + operator= (uuid&& u) noexcept { if (this != &u) { -- cgit v1.1