From a534f4a168eb42c3efdb732f22ca3cc686f8a2d7 Mon Sep 17 00:00:00 2001 From: Niels Dekker Date: Mon, 13 Jun 2022 18:09:54 +0200 Subject: [PATCH 1/6] F.16 ("in" parameters): Move Matrix example to F.20 (return values) (#1922) The `Matrix` example and the notes about assignment appear off-topic in rule F.16, as F.16 is specifically about "in" parameters. With help from Sergey Zubkov. --- CppCoreGuidelines.md | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 375cb08bf..0b884cc80 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -2927,31 +2927,11 @@ For advanced uses (only), where you really need to optimize for rvalues passed t void sink(unique_ptr); // input only, and moves ownership of the widget -Avoid "esoteric techniques" such as: - -* Passing arguments as `T&&` "for efficiency". - Most rumors about performance advantages from passing by `&&` are false or brittle (but see [F.18](#Rf-consume) and [F.19](#Rf-forward)). -* Returning `const T&` from assignments and similar operations (see [F.47](#Rf-assignment-op).) - -##### Example - -Assuming that `Matrix` has move operations (possibly by keeping its elements in a `std::vector`): - - Matrix operator+(const Matrix& a, const Matrix& b) - { - Matrix res; - // ... fill res with the sum ... - return res; - } - - Matrix x = m1 + m2; // move constructor - - y = m3 + m3; // move assignment +Avoid "esoteric techniques" such as passing arguments as `T&&` "for efficiency". +Most rumors about performance advantages from passing by `&&` are false or brittle (but see [F.18](#Rf-consume) and [F.19](#Rf-forward)). ##### Notes -The return value optimization doesn't handle the assignment case, but the move assignment does. - A reference can be assumed to refer to a valid object (language rule). There is no (legitimate) "null reference." If you need the notion of an optional value, use a pointer, `std::optional`, or a special value used to denote "no value." @@ -3104,6 +3084,26 @@ The argument against is that it prevents (very frequent) use of move semantics. * If a type is expensive to move (e.g., `array`), consider allocating it on the free store and return a handle (e.g., `unique_ptr`), or passing it in a reference to non-`const` target object to fill (to be used as an out-parameter). * To reuse an object that carries capacity (e.g., `std::string`, `std::vector`) across multiple calls to the function in an inner loop: [treat it as an in/out parameter and pass by reference](#Rf-out-multi). +##### Example + +Assuming that `Matrix` has move operations (possibly by keeping its elements in a `std::vector`): + + Matrix operator+(const Matrix& a, const Matrix& b) + { + Matrix res; + // ... fill res with the sum ... + return res; + } + + Matrix x = m1 + m2; // move constructor + + y = m3 + m3; // move assignment + + +##### Note + +The return value optimization doesn't handle the assignment case, but the move assignment does. + ##### Example struct Package { // exceptional case: expensive-to-move object From 9ead2c44b4bedf212aa4add84afead043110a73b Mon Sep 17 00:00:00 2001 From: Niels Dekker Date: Mon, 13 Jun 2022 18:11:50 +0200 Subject: [PATCH 2/6] SL.io.50 (Avoid `endl`): Mention string streams (#1920) Explicitly mentioned string streams as `endl` insertions into string streams do actually occur in the wild. With help from Sergey Zubkov. --- CppCoreGuidelines.md | 8 ++++++++ scripts/hunspell/isocpp.dic | 1 + 2 files changed, 9 insertions(+) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 0b884cc80..a252c9583 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -20308,6 +20308,14 @@ For writing to a file, there is rarely a need to `flush`. ##### Note +For string streams (specifically `ostringstream`), the insertion of an `endl` is entirely equivalent +to the insertion of a `'\n'` character, but also in this case, `endl` might be significantly slower. + +`endl` does *not* take care of producing a platform specific end-of-line sequence (like "\r\n" on +Windows). So for a string stream, `s << endl` just inserts a *single* character, `'\n'`. + +##### Note + Apart from the (occasionally important) issue of performance, the choice between `'\n'` and `endl` is almost completely aesthetic. diff --git a/scripts/hunspell/isocpp.dic b/scripts/hunspell/isocpp.dic index ef45e4ecd..e9d90e50f 100644 --- a/scripts/hunspell/isocpp.dic +++ b/scripts/hunspell/isocpp.dic @@ -389,6 +389,7 @@ optimizable O'Reilly org ostream +ostringstream overabstract overconstrain overconstrained From d5907d6dd505844237249f37c0a6f7cc02c9ab56 Mon Sep 17 00:00:00 2001 From: Herb Sutter Date: Mon, 13 Jun 2022 13:14:54 -0700 Subject: [PATCH 3/6] Extended E.16 to include copy ctor for exception type, closes #1921 --- CppCoreGuidelines.md | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index a252c9583..1510e3484 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -1,6 +1,6 @@ # C++ Core Guidelines -January 3, 2022 +June 13, 2022 Editors: @@ -15629,7 +15629,7 @@ Error-handling rule summary: * [E.13: Never throw while being the direct owner of an object](#Re-never-throw) * [E.14: Use purpose-designed user-defined types as exceptions (not built-in types)](#Re-exception-types) * [E.15: Throw by value, catch exceptions from a hierarchy by reference](#Re-exception-ref) -* [E.16: Destructors, deallocation, and `swap` must never fail](#Re-never-fail) +* [E.16: Destructors, deallocation, `swap`, and exception type copy/move construction must never fail](#Re-never-fail) * [E.17: Don't try to catch every exception in every function](#Re-not-always) * [E.18: Minimize the use of explicit `try`/`catch`](#Re-catch) * [E.19: Use a `final_action` object to express cleanup if no suitable resource handle is available](#Re-finally) @@ -16096,11 +16096,11 @@ To rethrow a caught exception use `throw;` not `throw e;`. Using `throw e;` woul * Flag catching by value of a type that has a virtual function. * Flag throwing raw pointers. -### E.16: Destructors, deallocation, and `swap` must never fail +### E.16: Destructors, deallocation, `swap`, and exception type copy/move construction must never fail ##### Reason -We don't know how to write reliable programs if a destructor, a swap, or a memory deallocation fails; that is, if it exits by an exception or simply doesn't perform its required action. +We don't know how to write reliable programs if a destructor, a swap, a memory deallocation, or attempting to copy/move-construct an exception object fails; that is, if it exits by an exception or simply doesn't perform its required action. ##### Example, don't @@ -16129,14 +16129,17 @@ The standard library assumes that destructors, deallocation functions (e.g., `op ##### Note -Deallocation functions, including `operator delete`, must be `noexcept`. `swap` functions must be `noexcept`. -Most destructors are implicitly `noexcept` by default. -Also, [make move operations `noexcept`](#Rc-move-noexcept). +- Deallocation functions, including `operator delete`, must be `noexcept`. +- `swap` functions must be `noexcept`. +- Most destructors are implicitly `noexcept` by default. +- Also, [make move operations `noexcept`](#Rc-move-noexcept). +- If writing a type intended to be used as an exception type, ensure its copy constructor is not `noexcept`. In general we cannot mechanically enforce this, because we do not know whether a type is intended to be used as an exception type. +- Try not to `throw` a type whose copy constructor is not `noexcept`. In general we cannot mechanically enforce this, because even `throw std::string(...)` could throw but does not in practice. ##### Enforcement -Catch destructors, deallocation operations, and `swap`s that `throw`. -Catch such operations that are not `noexcept`. +- Catch destructors, deallocation operations, and `swap`s that `throw`. +- Catch such operations that are not `noexcept`. **See also**: [discussion](#Sd-never-fail) From 81dfb4814d06fcb9c4e1d859f4288544869dc5d0 Mon Sep 17 00:00:00 2001 From: Niels Dekker Date: Wed, 15 Jun 2022 16:01:28 +0200 Subject: [PATCH 4/6] Fix GitHub Actions build warnings, Marker style should be `*` (#1925) --- CppCoreGuidelines.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 1510e3484..26c6a9e7f 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -16129,17 +16129,17 @@ The standard library assumes that destructors, deallocation functions (e.g., `op ##### Note -- Deallocation functions, including `operator delete`, must be `noexcept`. -- `swap` functions must be `noexcept`. -- Most destructors are implicitly `noexcept` by default. -- Also, [make move operations `noexcept`](#Rc-move-noexcept). -- If writing a type intended to be used as an exception type, ensure its copy constructor is not `noexcept`. In general we cannot mechanically enforce this, because we do not know whether a type is intended to be used as an exception type. -- Try not to `throw` a type whose copy constructor is not `noexcept`. In general we cannot mechanically enforce this, because even `throw std::string(...)` could throw but does not in practice. +* Deallocation functions, including `operator delete`, must be `noexcept`. +* `swap` functions must be `noexcept`. +* Most destructors are implicitly `noexcept` by default. +* Also, [make move operations `noexcept`](#Rc-move-noexcept). +* If writing a type intended to be used as an exception type, ensure its copy constructor is not `noexcept`. In general we cannot mechanically enforce this, because we do not know whether a type is intended to be used as an exception type. +* Try not to `throw` a type whose copy constructor is not `noexcept`. In general we cannot mechanically enforce this, because even `throw std::string(...)` could throw but does not in practice. ##### Enforcement -- Catch destructors, deallocation operations, and `swap`s that `throw`. -- Catch such operations that are not `noexcept`. +* Catch destructors, deallocation operations, and `swap`s that `throw`. +* Catch such operations that are not `noexcept`. **See also**: [discussion](#Sd-never-fail) From e2ab9f5b592eaebc49dce6f26f70a950fdfb1496 Mon Sep 17 00:00:00 2001 From: bgloyer <36457894+bgloyer@users.noreply.github.com> Date: Wed, 15 Jun 2022 21:52:16 -0700 Subject: [PATCH 5/6] restored reference --- CppCoreGuidelines.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 99c577ea1..ff993fea6 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -4642,7 +4642,7 @@ Destructor rules: * [C.30: Define a destructor if a class needs an explicit action at object destruction](#Rc-dtor) * [C.31: All resources acquired by a class must be released by the class's destructor](#Rc-dtor-release) -* [C.32: If a class has a raw pointer (`T*`), consider whether it might be owning](#Rc-dtor-ptr) +* [C.32: If a class has a raw pointer (`T*`) or reference (`T&`), consider whether it might be owning](#Rc-dtor-ptr) * [C.33: If a class has an owning pointer member, define a destructor](#Rc-dtor-ptr2) * [C.35: A base class destructor should be either public and virtual, or protected and non-virtual](#Rc-dtor-virtual) * [C.36: A destructor must not fail](#Rc-dtor-fail) @@ -4985,7 +4985,7 @@ Here `p` refers to `pp` but does not own it. * (Hard) Determine if pointer or reference member variables are owners when there is no explicit statement of ownership (e.g., look into the constructors). -### C.32: If a class has a raw pointer (`T*`), consider whether it might be owning +### C.32: If a class has a raw pointer (`T*`) or reference (`T&`), consider whether it might be owning ##### Reason @@ -5000,7 +5000,7 @@ There is a lot of code that is non-specific about ownership. } The only way to determine ownership may be to dig through the code to look for -allocations. If a pointer is owning, document it as owning. +allocations. If a pointer or reference is owning, document it as owning. ##### Note @@ -5009,7 +5009,7 @@ resources and [R.3](#Rr-ptr) for non-owned resources. ##### Enforcement -Look at the initialization of raw member pointers and see if an allocation is used. +Look at the initialization of raw member pointers and member references and see if an allocation is used. ### C.33: If a class has an owning pointer member, define a destructor From ea7ecc5a21659666302b509b77537dd9f395d891 Mon Sep 17 00:00:00 2001 From: bgloyer <36457894+bgloyer@users.noreply.github.com> Date: Wed, 15 Jun 2022 22:01:20 -0700 Subject: [PATCH 6/6] Added references to note --- CppCoreGuidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index ff993fea6..bf1df0297 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -5005,7 +5005,7 @@ allocations. If a pointer or reference is owning, document it as owning. ##### Note Ownership should be clear in new code (and refactored legacy code) according to [R.20](#Rr-owner) for owned -resources and [R.3](#Rr-ptr) for non-owned resources. +pointers and [R.3](#Rr-ptr) for non-owned pointers. References should never own [R.4](#Rr-ref). ##### Enforcement