From 1ebedb876fdb4e49a27684f92fc53eccdab2dc24 Mon Sep 17 00:00:00 2001 From: bgloyer <36457894+bgloyer@users.noreply.github.com> Date: Tue, 3 May 2022 20:10:49 -0700 Subject: [PATCH 1/6] Update CppCoreGuidelines.md --- CppCoreGuidelines.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 5d2104d1d..30256fc31 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*`) or reference (`T&`), consider whether it might be owning](#Rc-dtor-ptr) +* [C.32: If a class has a raw pointer (`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*`) or reference (`T&`), consider whether it might be owning +### C.32: If a class has a raw pointer (`T*`), consider whether it might be owning ##### Reason @@ -4993,16 +4993,23 @@ There is a lot of code that is non-specific about ownership. ##### Example - ??? + class LegacyClass + { + Foo* m_owningPtr; + Bar* m_observerPtr; + } + +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. ##### Note -If the `T*` or `T&` is owning, mark it `owning`. If the `T*` is not owning, consider marking it `ptr`. -This will aid documentation and analysis. +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. ##### Enforcement -Look at the initialization of raw member pointers and member references and see if an allocation is used. +Look at the initialization of raw member pointers and see if an allocation is used. ### C.33: If a class has an owning pointer member, define a destructor From decef1813a59c55d1e3d7363c39e9a976ee350a8 Mon Sep 17 00:00:00 2001 From: bgloyer <36457894+bgloyer@users.noreply.github.com> Date: Tue, 3 May 2022 20:14:17 -0700 Subject: [PATCH 2/6] Update CppCoreGuidelines.md --- CppCoreGuidelines.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 30256fc31..bbe6d4381 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -4997,6 +4997,7 @@ There is a lot of code that is non-specific about ownership. { Foo* m_owningPtr; Bar* m_observerPtr; + // ... } The only way to determine ownership may be to dig through the code to look for From 05100a9fec6a7893b32d5223d0c801f9a0271266 Mon Sep 17 00:00:00 2001 From: bgloyer <36457894+bgloyer@users.noreply.github.com> Date: Tue, 3 May 2022 20:20:35 -0700 Subject: [PATCH 3/6] Update isocpp.dic --- scripts/hunspell/isocpp.dic | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/hunspell/isocpp.dic b/scripts/hunspell/isocpp.dic index ef45e4ecd..f9d366cbb 100644 --- a/scripts/hunspell/isocpp.dic +++ b/scripts/hunspell/isocpp.dic @@ -286,6 +286,7 @@ Lakos Lakos96 Lavavej LCSD05 +LegacyClass lifecycle *life-time linearization @@ -301,6 +302,8 @@ lvalues m1 m2 m3 +m_owningPtr; +m_observerPtr; macros2 malloc mallocfree From 445945f03d24e2ef045f5cec78ce50e8537d805e Mon Sep 17 00:00:00 2001 From: bgloyer <36457894+bgloyer@users.noreply.github.com> Date: Sun, 5 Jun 2022 21:28:46 -0700 Subject: [PATCH 4/6] use snake casing --- CppCoreGuidelines.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index d14586d56..eabfb0e9d 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -4993,11 +4993,10 @@ There is a lot of code that is non-specific about ownership. ##### Example - class LegacyClass + class legacy_class { - Foo* m_owningPtr; - Bar* m_observerPtr; - // ... + foo* m_owning; + bar* m_observer; } The only way to determine ownership may be to dig through the code to look for From e0aec519d5c218857242760ece37ec207cfcbbac Mon Sep 17 00:00:00 2001 From: bgloyer <36457894+bgloyer@users.noreply.github.com> Date: Sun, 5 Jun 2022 21:32:52 -0700 Subject: [PATCH 5/6] sake case naming --- scripts/hunspell/isocpp.dic | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/hunspell/isocpp.dic b/scripts/hunspell/isocpp.dic index f9d366cbb..a60cc33b3 100644 --- a/scripts/hunspell/isocpp.dic +++ b/scripts/hunspell/isocpp.dic @@ -286,7 +286,7 @@ Lakos Lakos96 Lavavej LCSD05 -LegacyClass +legacy_class lifecycle *life-time linearization @@ -302,8 +302,8 @@ lvalues m1 m2 m3 -m_owningPtr; -m_observerPtr; +m_owning; +m_observer; macros2 malloc mallocfree From 9acf9ac99441306b4e4ab74e3fc1e2714a8dc235 Mon Sep 17 00:00:00 2001 From: bgloyer <36457894+bgloyer@users.noreply.github.com> Date: Wed, 15 Jun 2022 22:09:33 -0700 Subject: [PATCH 6/6] C 32 comments (#3) * 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. * 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. * Extended E.16 to include copy ctor for exception type, closes #1921 * Fix GitHub Actions build warnings, Marker style should be `*` (#1925) * restored reference * Added references to note Co-authored-by: Niels Dekker Co-authored-by: Herb Sutter --- CppCoreGuidelines.md | 83 +++++++++++++++++++++---------------- scripts/hunspell/isocpp.dic | 1 + 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index eabfb0e9d..bf1df0297 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -1,6 +1,6 @@ # C++ Core Guidelines -January 3, 2022 +June 13, 2022 Editors: @@ -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 @@ -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,16 +5000,16 @@ 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 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 -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 @@ -15636,7 +15636,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) @@ -16103,11 +16103,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 @@ -16136,14 +16136,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) @@ -20315,6 +20318,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 a60cc33b3..0b3520d19 100644 --- a/scripts/hunspell/isocpp.dic +++ b/scripts/hunspell/isocpp.dic @@ -392,6 +392,7 @@ optimizable O'Reilly org ostream +ostringstream overabstract overconstrain overconstrained