From a968af59d0be0047a2fcf7f6697e0cad8e8b9b8e Mon Sep 17 00:00:00 2001 From: Sergey Zubkov Date: Sun, 18 Dec 2016 14:46:11 -0500 Subject: [PATCH 1/5] C.138 initial content --- CppCoreGuidelines.md | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 8df991e..f861b15 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -6561,11 +6561,49 @@ This a relatively rare use because implementation can often be organized into a ##### Reason -??? +Without a using declaration, member functions in the derived class hide the entire inherited overload sets. -##### Example +##### Example, bad - ??? + #include + class B { + public: + virtual int f(int i) { std::cout << "f(int): "; return i; } + virtual double f(double d) { std::cout << "f(double): "; return d; } + }; + class D: public B { + public: + int f(int i) override { std::cout << "f(int): "; return i+1; } + }; + int main() + { + D d; + std::cout << d.f(2) << '\n'; // prints "f(int): 3" + std::cout << d.f(2.3) << '\n'; // prints "f(int): 3" + } + +##### Example, good + + class D: public B { + public: + int f(int i) override { std::cout << "f(int): "; return i+1; } + using B::f; // exposes f(double) + }; + +##### Note + +This issue affects both virtual and non-virtual member functions + +For variadic bases, C++17 introduced a variadic form of the using-declaration, + + template + struct Overloader : Ts... { + using Ts::operator()...; // exposes operator() from every base + }; + +##### Enforcement + +Diagnose name hiding ### C.139: Use `final` sparingly From 51b4cddbca83d26529fdecda48a2bfb4cee40821 Mon Sep 17 00:00:00 2001 From: hsutter Date: Mon, 30 Jan 2017 11:40:43 -0800 Subject: [PATCH 2/5] Closed #810 Added an example to SF.7 Clarified SF.6 and SF.7 that "using-directive" means "using namespace" --- CppCoreGuidelines.md | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 770c235..2ef163b 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -16068,8 +16068,8 @@ Source file rule summary: * [SF.3: Use `.h` files for all declarations used in multiple source files](#Rs-declaration-header) * [SF.4: Include `.h` files before other declarations in a file](#Rs-include-order) * [SF.5: A `.cpp` file must include the `.h` file(s) that defines its interface](#Rs-consistency) -* [SF.6: Use `using`-directives for transition, for foundation libraries (such as `std`), or within a local scope](#Rs-using) -* [SF.7: Don't put a `using`-directive in a header file](#Rs-using-directive) +* [SF.6: Use `using namespace` directives for transition, for foundation libraries (such as `std`), or within a local scope](#Rs-using) +* [SF.7: Don't write `using namespace` in a header file](#Rs-using-directive) * [SF.8: Use `#include` guards for all `.h` files](#Rs-guards) * [SF.9: Avoid cyclic dependencies among source files](#Rs-cycles) @@ -16271,7 +16271,7 @@ The argument-type error for `bar` cannot be caught until link time because of th ??? -### SF.6: Use `using`-directives for transition, for foundation libraries (such as `std`), or within a local scope +### SF.6: Use `using namespace` directives for transition, for foundation libraries (such as `std`), or within a local scope ##### Reason @@ -16285,7 +16285,7 @@ The argument-type error for `bar` cannot be caught until link time because of th ??? -### SF.7: Don't put a `using`-directive in a header file +### SF.7: Don't write `using namespace` in a header file ##### Reason @@ -16293,11 +16293,22 @@ Doing so takes away an `#include`r's ability to effectively disambiguate and to ##### Example - ??? + // bad.h + #include + using namespace std; // bad + + // user.cpp + #include "bad.h" + + bool copy( /*... some parameters ...*/); // some function that happens to be named copy + + int main() { + copy( /*...*/ ); // now overloads local ::copy and std::copy, could be ambiguous + } ##### Enforcement -??? +Flag `using namespace` at global scope in a header file. ### SF.8: Use `#include` guards for all `.h` files From 8f2ecf0665777dc137347cee14b48b6e99a58290 Mon Sep 17 00:00:00 2001 From: hsutter Date: Mon, 30 Jan 2017 11:52:11 -0800 Subject: [PATCH 3/5] Closed #557 Added example from the issue --- CppCoreGuidelines.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 2ef163b..4b0a200 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -9613,7 +9613,7 @@ not. Unfortunately, it may be impossible to detect when a non-`const` was not ##### Reason -Readability. +Readability and safety. ##### Example, bad @@ -9624,6 +9624,26 @@ Readability. for (i = 0; i < 200; ++i) { /* ... */ } // bad: i recycled } +##### Note + +As an optimization, you may want to reuse a buffer as a scratchpad, but even then prefer to limit the variables's scope as much as possible and be careful not to cause bugs from data left in a recycled buffer as this is a common source of security bugs. + + { + std::string buffer; // to avoid reallocations on every loop iteration + for (auto& o : objects) + { + // First part of the work. + generateFirstString(buffer, o); + writeToFile(buffer); + + // Second part of the work. + generateSecondString(buffer, o); + writeToFile(buffer); + + // etc... + } + } + ##### Enforcement Flag recycled variables. From 14f67ae8dd65803165e85b703370509feb809566 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Tue, 31 Jan 2017 12:13:10 +0000 Subject: [PATCH 4/5] Fix typos from commit 9858685786c25d3bbd1b1ea318f9de0ce23dee50 --- CppCoreGuidelines.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 4b0a200..b338f63 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -3774,17 +3774,17 @@ Concrete types are also often referred to as value types to distinguish them fro Concrete type rule summary: -* [C.10: Prefer a concrete types over class hierarchies](#Rc-concrete) +* [C.10: Prefer concrete types over class hierarchies](#Rc-concrete) * [C.11: Make concrete types regular](#Rc-regular) -### C.10 Prefer a concrete types over class hierarchies +### C.10 Prefer concrete types over class hierarchies ##### Reason A concrete type is fundamentally simpler than a hierarchy: easier to design, easier to implement, easier to use, easier to reason about, smaller, and faster. You need a reason (use cases) for using a hierarchy. -n + ##### Example class Point1 { From f2dfcfc43839e0a623665e03b55527e0a4fec890 Mon Sep 17 00:00:00 2001 From: Bjarne Stroustrup Date: Wed, 1 Feb 2017 17:05:22 -0500 Subject: [PATCH 5/5] minor cleanup --- CppCoreGuidelines.md | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index b338f63..e97d9bd 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -1,6 +1,6 @@ # C++ Core Guidelines -December 12, 2016 +February 1, 2017 Editors: @@ -6388,7 +6388,7 @@ A trivial getter or setter adds no semantic value; the data item could just as w ##### Example - class Point { + class Point { // Bad: verbose int x; int y; public: @@ -6403,10 +6403,12 @@ A trivial getter or setter adds no semantic value; the data item could just as w Consider making such a class a `struct` -- that is, a behaviorless bunch of variables, all public data and no member functions. struct Point { - int x = 0; - int y = 0; + int x {0}; + int y {0}; }; +Note that we can put default initializers on member variables: [C.49: Prefer initialization to assignment in constructors](#Rc-initialize). + ##### Note A getter or a setter that converts from an internal type to an interface type is not trivial (it provides a form of information hiding). @@ -9316,7 +9318,7 @@ Assuming that there is a logical connection between `i` and `j`, that connection Obviously, what we really would like is a construct that initialized n variables from a `tuple`. For example: - auto {i, j} = make_related_widgets(cond); // Not C++14 + auto [i,j] = make_related_widgets(cond); // C++17, not C++14 Today, we might approximate that using `tie()`: @@ -13782,7 +13784,28 @@ This gives a more precise statement of design intent, better readability, more e ##### Note -[Do not cast away `const`](#Res-casts-const). +It is not inherently bad to pass a pointer or reference to non-const, +but that should be done only when the called function is supposed to modify the object. +A reader of code must assume that a funtion that takes a "plain" `T*` or `T&` will modify the object referred to. +If it doesn't now, it might do so later without forcing recompilation. + +##### Note + +There are code/libraries that are offer functions that declare a`T*` even though +those function do not modify that `T`. +This is a problem for people modernizing code. +You can + +* update the library to be `const`-correct; preferred long-term solution +* "cast away `const`"; [best avoided](#Res-casts-const). +* provide a wrapper function; for example + + void f(int* p); // old code: f() does not mpdify `*p` + void f(const int* p) { f(const_cast(p); } // wrapper + +Note that this wrapper solution is a patch that should be used only when the declaration of `f()` cannot be be modified, +e.g. because it is in a library that you cannot modify. + ##### Enforcement @@ -19152,10 +19175,11 @@ A relatively informal definition of terms used in the guidelines This is our to-do list. Eventually, the entries will become rules or parts of rules. Alternatively, we will decide that no change is needed and delete the entry. - * No long-distance friendship * Should physical design (what's in a file) and large-scale design (libraries, groups of libraries) be addressed? * Namespaces +* Don't place using directives in headers +* Avoid using directives in the global scope (except for std, and other "fundamental" namespaces (e.g. experimental)) * How granular should namespaces be? All classes/functions designed to work together and released together (as defined in Sutter/Alexandrescu) or something narrower or wider? * Should there be inline namespaces (à la `std::literals::*_literals`)? * Avoid implicit conversions