Merge pull request #210 from tkruse/fix-mdstyle10

Fix style and code examples
This commit is contained in:
Gabriel Dos Reis 2015-09-29 08:40:23 -07:00
commit 6553f69976

View File

@ -128,7 +128,7 @@ Comments and suggestions for improvements are most welcome.
We plan to modify and extend this document as our understanding improves and the language and the set of available libraries improve. We plan to modify and extend this document as our understanding improves and the language and the set of available libraries improve.
# <a name ="S-introduction"></a> In: Introduction # <a name="S-introduction"></a> In: Introduction
This is a set of core guidelines for modern C++, C++14, and taking likely future enhancements and taking ISO Technical Specifications (TSs) into account. This is a set of core guidelines for modern C++, C++14, and taking likely future enhancements and taking ISO Technical Specifications (TSs) into account.
The aim is to help C++ programmers to write simpler, more efficient, more maintainable code. The aim is to help C++ programmers to write simpler, more efficient, more maintainable code.
@ -143,12 +143,12 @@ Introduction summary:
* [In.sec: Major sections](#SS-sec) * [In.sec: Major sections](#SS-sec)
## <a name ="SS-readers"></a> In.target: Target readership ## <a name="SS-readers"></a> In.target: Target readership
All C++ programmers. This includes [programmers who might consider C](#S-cpl). All C++ programmers. This includes [programmers who might consider C](#S-cpl).
## <a name ="SS-aims"></a> In.aims: Aims ## <a name="SS-aims"></a> In.aims: Aims
The purpose of this document is to help developers to adopt modern C++ (C++11, C++14, and soon C++17) and to achieve a more uniform style across code bases. The purpose of this document is to help developers to adopt modern C++ (C++11, C++14, and soon C++17) and to achieve a more uniform style across code bases.
@ -261,7 +261,7 @@ The profiles are intended to be used by tools, but also serve as an aid to the h
We do not limit our comment in the **Enforcement** sections to things we know how to enforce; some comments are mere wishes that might inspire some tool builder. We do not limit our comment in the **Enforcement** sections to things we know how to enforce; some comments are mere wishes that might inspire some tool builder.
## <a name ="SS-struct"></a> In.struct: The structure of this document ## <a name="SS-struct"></a> In.struct: The structure of this document
Each rule (guideline, suggestion) can have several parts: Each rule (guideline, suggestion) can have several parts:
@ -292,7 +292,7 @@ It is meant to be helpful, rather than complete, fully accurate on technical det
Recommended information sources can be found in [the references](#S-references). Recommended information sources can be found in [the references](#S-references).
## <a name ="SS-sec"></a> In.sec: Major sections ## <a name="SS-sec"></a> In.sec: Major sections
* [P: Philosophy](#S-philosophy) * [P: Philosophy](#S-philosophy)
* [I: Interfaces](#S-interfaces) * [I: Interfaces](#S-interfaces)
@ -324,7 +324,7 @@ Supporting sections:
These sections are not orthogonal. These sections are not orthogonal.
Each section (e.g., "P" for "Philosophy") and each subsection (e.g., "C.hier" for "Class Hierachies (OOP)") have an abbreviation for ease of searching and reference. Each section (e.g., "P" for "Philosophy") and each subsection (e.g., "C.hier" for "Class Hierarchies (OOP)") have an abbreviation for ease of searching and reference.
The main section abbreviations are also used in rule numbers (e.g., "C.11" for "Make concrete types regular"). The main section abbreviations are also used in rule numbers (e.g., "C.11" for "Make concrete types regular").
@ -718,23 +718,23 @@ The date is validated twice (by the `Date` constructor) and passed as a characte
There are cases where checking early is dumb because you may not ever need the value, There are cases where checking early is dumb because you may not ever need the value,
or may only need part of the value that is more easily checked than the whole. or may only need part of the value that is more easily checked than the whole.
class Jet { // Physics says: e*e < x*x + y*y + z*z class Jet { // Physics says: e*e < x*x + y*y + z*z
float fx, fy, fz, fe; float fx, fy, fz, fe;
public: public:
Jet(float x, float y, float z, float e) Jet(float x, float y, float z, float e)
:fx(x), fy(y), fz(z), fe(e) :fx(x), fy(y), fz(z), fe(e)
{ {
// Should I check the here that the values are physically meaningful? // Should I check the here that the values are physically meaningful?
} }
float m() const float m() const
{ {
// Should I handle the degenerate case here? // Should I handle the degenerate case here?
return sqrt(x*x + y*y + z*z - e*e); return sqrt(x*x + y*y + z*z - e*e);
} }
??? ???
}; };
The physical law for a jet (`e*e < x*x + y*y + z*z`) is not an invariant because the possibility of measurement errors. The physical law for a jet (`e*e < x*x + y*y + z*z`) is not an invariant because the possibility of measurement errors.
@ -2158,7 +2158,7 @@ If the writer of `g()` makes an assumption about the size of `buffer` a bad logi
**Enforcement**: Flag a function that takes a `TP&&` parameter (where `TP` is a template type parameter name) and uses it without `std::forward`. **Enforcement**: Flag a function that takes a `TP&&` parameter (where `TP` is a template type parameter name) and uses it without `std::forward`.
### <a name ="Rf-pass-ref-move"></a> F.25: Use a `T&&` parameter together with `move` for rare optimization opportunities ### <a name="Rf-pass-ref-move"></a> F.25: Use a `T&&` parameter together with `move` for rare optimization opportunities
**Reason**: Moving from an object leaves an object in its moved-from state behind. **Reason**: Moving from an object leaves an object in its moved-from state behind.
In general, moved-from objects are dangerous. The only guaranteed operation is destruction (more generally, member functions without preconditions). In general, moved-from objects are dangerous. The only guaranteed operation is destruction (more generally, member functions without preconditions).
@ -2585,8 +2585,8 @@ Subsections:
**Example**: **Example**:
void draw(int x, int y, int x2, int y2); // BAD: unnecessary implicit relationships void draw(int x, int y, int x2, int y2); // BAD: unnecessary implicit relationships
void draw(Point from, Point to) // better void draw(Point from, Point to); // better
**Note**: A simple class without virtual functions implies no space or time overhead. **Note**: A simple class without virtual functions implies no space or time overhead.
@ -3024,7 +3024,7 @@ The whole purpose of `final_action` is to get a piece of code (usually a lambda)
string s; string s;
int i; int i;
vector<int> vi; vector<int> vi;
} };
The default destructor does it better, more efficiently, and can't get it wrong. The default destructor does it better, more efficiently, and can't get it wrong.
@ -3353,7 +3353,7 @@ It is often a good idea to express the invariant as an `Ensure` on the construct
struct Rec2{ struct Rec2{
string s; string s;
int i; int i;
Rec2(const string& ss, int ii = 0} :s{ss}, i{ii} {} // redundant Rec2(const string& ss, int ii = 0) :s{ss}, i{ii} {} // redundant
}; };
Rec r1 {"Foo", 7}; Rec r1 {"Foo", 7};
@ -3413,7 +3413,7 @@ The idiom of having constructors acquire resources and destructors release them
// ... // ...
public: public:
X2(const string& name) X2(const string& name)
:f{fopen(name.c_str(), "r"} :f{fopen(name.c_str(), "r")}
{ {
if (f==nullptr) throw runtime_error{"could not open" + name}; if (f==nullptr) throw runtime_error{"could not open" + name};
// ... // ...
@ -3438,7 +3438,7 @@ The idiom of having constructors acquire resources and destructors release them
// ... // ...
public: public:
X3(const string& name) X3(const string& name)
:f{fopen(name.c_str(), "r"}, valid{false} :f{fopen(name.c_str(), "r")}, valid{false}
{ {
if (f) valid=true; if (f) valid=true;
// ... // ...
@ -3451,7 +3451,7 @@ The idiom of having constructors acquire resources and destructors release them
void f() void f()
{ {
X3 file {Heraclides"}; X3 file {"Heraclides"};
file.read(); // crash or bad read! file.read(); // crash or bad read!
// ... // ...
if (is_valid()()) { if (is_valid()()) {
@ -3881,6 +3881,7 @@ Types can be defined to move for logical as well as performance reasons.
T* elem; T* elem;
int sz; int sz;
}; };
}
Vector& Vector::operator=(const Vector& a) Vector& Vector::operator=(const Vector& a)
{ {
@ -4049,6 +4050,7 @@ Consider:
**Example**: **Example**:
template<typename T>
class X { // OK: value semantics class X { // OK: value semantics
public: public:
X(); X();
@ -4134,6 +4136,7 @@ A non-throwing move will be used more efficiently by standard-library and langua
**Example**: **Example**:
template<typename T>
class Vector { class Vector {
// ... // ...
Vector(Vector&& a) noexcept :elem{a.elem}, sz{a.sz} { a.sz=0; a.elem=nullptr; } Vector(Vector&& a) noexcept :elem{a.elem}, sz{a.sz} { a.sz=0; a.elem=nullptr; }
@ -4148,6 +4151,7 @@ These copy operations do not throw.
**Example, bad**: **Example, bad**:
template<typename T>
class Vector2 { class Vector2 {
// ... // ...
Vector2(Vector2&& a) { *this = a; } // just use the copy Vector2(Vector2&& a) { *this = a; } // just use the copy
@ -5333,7 +5337,7 @@ Here, we ignore such cases.
* [R.14: ??? array vs. pointer parameter](#Rr-ap) * [R.14: ??? array vs. pointer parameter](#Rr-ap)
* [R.15: Always overload matched allocation/deallocation pairs](#Rr-pair) * [R.15: Always overload matched allocation/deallocation pairs](#Rr-pair)
* <a name ="Rr-summary-smartptrs"></a> Smart pointer rule summary: * <a name="Rr-summary-smartptrs"></a> Smart pointer rule summary:
* [R.20: Use `unique_ptr` or `shared_ptr` to represent ownership](#Rr-owner) * [R.20: Use `unique_ptr` or `shared_ptr` to represent ownership](#Rr-owner)
* [R.21: Prefer `unique_ptr` over `shared_ptr` unless you need to share ownership](#Rr-unique) * [R.21: Prefer `unique_ptr` over `shared_ptr` unless you need to share ownership](#Rr-unique)
* [R.22: Use `make_shared()` to make `shared_ptr`s](#Rr-make_shared) * [R.22: Use `make_shared()` to make `shared_ptr`s](#Rr-make_shared)
@ -5348,7 +5352,7 @@ Here, we ignore such cases.
* [R.36: Take a `const shared_ptr<widget>&` parameter to express that it might retain a reference count to the object ???](#Rr-sharedptrparam-const&) * [R.36: Take a `const shared_ptr<widget>&` parameter to express that it might retain a reference count to the object ???](#Rr-sharedptrparam-const&)
* [R.37: Do not pass a pointer or reference obtained from an aliased smart pointer](#Rr-smartptrget) * [R.37: Do not pass a pointer or reference obtained from an aliased smart pointer](#Rr-smartptrget)
### <a name ="Rr-raii"></a> Rule R.1: Manage resources automatically using resource handles and RAII (resource acquisition is initialization) ### <a name="Rr-raii"></a> Rule R.1: Manage resources automatically using resource handles and RAII (resource acquisition is initialization)
**Reason**: To avoid leaks and the complexity of manual resource management. **Reason**: To avoid leaks and the complexity of manual resource management.
C++'s language-enforced constructor/destructor symmetry mirrors the symmetry inherent in resource acquire/release function pairs such as `fopen`/`fclose`, `lock`/`unlock`, and `new`/`delete`. C++'s language-enforced constructor/destructor symmetry mirrors the symmetry inherent in resource acquire/release function pairs such as `fopen`/`fclose`, `lock`/`unlock`, and `new`/`delete`.
@ -5402,7 +5406,7 @@ What is `Port`? A handy wrapper that encapsulates the resource:
**See also**: [RAII](#Rr-raii). **See also**: [RAII](#Rr-raii).
### <a name ="Rr-use-ptr"></a> R.2: In interfaces, use raw pointers to denote individual objects (only) ### <a name="Rr-use-ptr"></a> R.2: In interfaces, use raw pointers to denote individual objects (only)
**Reason**: Arrays are best represented by a container type (e.g., `vector` (owning)) or an `array_view` (non-owning). **Reason**: Arrays are best represented by a container type (e.g., `vector` (owning)) or an `array_view` (non-owning).
Such containers and views hold sufficient information to do range checking. Such containers and views hold sufficient information to do range checking.
@ -5592,7 +5596,7 @@ They are a notable source of errors.
## <a name="SS-alloc"></a> R.alloc: Allocation and deallocation ## <a name="SS-alloc"></a> R.alloc: Allocation and deallocation
### <a name ="Rr-mallocfree"></a> R.10: Avoid `malloc()` and `free()` ### <a name="Rr-mallocfree"></a> R.10: Avoid `malloc()` and `free()`
**Reason**: `malloc()` and `free()` do not support construction and destruction, and do not mix well with `new` and `delete`. **Reason**: `malloc()` and `free()` do not support construction and destruction, and do not mix well with `new` and `delete`.
@ -5633,7 +5637,7 @@ In such cases, consider the `nothrow` versions of `new`.
**Enforcement**: Flag explicit use of `malloc` and `free`. **Enforcement**: Flag explicit use of `malloc` and `free`.
### <a name ="Rr-newdelete"></a> R.11: Avoid calling `new` and `delete` explicitly ### <a name="Rr-newdelete"></a> R.11: Avoid calling `new` and `delete` explicitly
**Reason**: The pointer returned by `new` should belong to a resource handle (that can call `delete`). **Reason**: The pointer returned by `new` should belong to a resource handle (that can call `delete`).
If the pointer returned from `new` is assigned to a plain/naked pointer, the object can be leaked. If the pointer returned from `new` is assigned to a plain/naked pointer, the object can be leaked.
@ -5646,7 +5650,7 @@ If you have a naked `new`, you probably need a naked `delete` somewhere, so you
**Enforcement**: (Simple) Warn on any explicit use of `new` and `delete`. Suggest using `make_unique` instead. **Enforcement**: (Simple) Warn on any explicit use of `new` and `delete`. Suggest using `make_unique` instead.
### <a name ="Rr-immediate-alloc"></a> R.12: Immediately give the result of an explicit resource allocation to a manager object ### <a name="Rr-immediate-alloc"></a> R.12: Immediately give the result of an explicit resource allocation to a manager object
**Reason**: If you don't, an exception or a return may lead to a leak. **Reason**: If you don't, an exception or a return may lead to a leak.
@ -5678,7 +5682,7 @@ The use of the file handle (in `ifstream`) is simple, efficient, and safe.
* Flag explicit allocations used to initialize pointers (problem: how many direct resource allocations can we recognize?) * Flag explicit allocations used to initialize pointers (problem: how many direct resource allocations can we recognize?)
### <a name ="Rr-single-alloc"></a> R.13: Perform at most one explicit resource allocation in a single expression statement ### <a name="Rr-single-alloc"></a> R.13: Perform at most one explicit resource allocation in a single expression statement
**Reason**: If you perform two explicit resource allocations in one statement, **Reason**: If you perform two explicit resource allocations in one statement,
you could leak resources because the order of evaluation of many subexpressions, including function arguments, is unspecified. you could leak resources because the order of evaluation of many subexpressions, including function arguments, is unspecified.
@ -5713,7 +5717,7 @@ Write your own factory wrapper if there is not one already.
* Flag expressions with multiple explicit resource allocations (problem: how many direct resource allocations can we recognize?) * Flag expressions with multiple explicit resource allocations (problem: how many direct resource allocations can we recognize?)
### <a name ="Rr-ap"></a> R.14: ??? array vs. pointer parameter ### <a name="Rr-ap"></a> R.14: ??? array vs. pointer parameter
**Reason**: An array decays to a pointer, thereby losing its size, opening the opportunity for range errors. **Reason**: An array decays to a pointer, thereby losing its size, opening the opportunity for range errors.
@ -5791,7 +5795,7 @@ This will leak the object used to initialize `p1` (only).
**Enforcement**: (Simple) Warn if a function uses a `Shared_ptr` with an object allocated within the function, but never returns the `Shared_ptr` or passes it to a function requiring a `Shared_ptr&`. Suggest using `unique_ptr` instead. **Enforcement**: (Simple) Warn if a function uses a `Shared_ptr` with an object allocated within the function, but never returns the `Shared_ptr` or passes it to a function requiring a `Shared_ptr&`. Suggest using `unique_ptr` instead.
### <a name ="Rr-make_shared"></a> R.22: Use `make_shared()` to make `shared_ptr`s ### <a name="Rr-make_shared"></a> R.22: Use `make_shared()` to make `shared_ptr`s
**Reason**: If you first make an object and then gives it to a `shared_ptr` constructor, you (most likely) do one more allocation (and later deallocation) than if you use `make_shared()` because the reference counts must be allocated separately from the object. **Reason**: If you first make an object and then gives it to a `shared_ptr` constructor, you (most likely) do one more allocation (and later deallocation) than if you use `make_shared()` because the reference counts must be allocated separately from the object.
@ -5805,7 +5809,7 @@ The `make_shared()` version mentions `X` only once, so it is usually shorter (as
**Enforcement**: (Simple) Warn if a `shared_ptr` is constructed from the result of `new` rather than `make_shared`. **Enforcement**: (Simple) Warn if a `shared_ptr` is constructed from the result of `new` rather than `make_shared`.
### <a name ="Rr-make_unique"></a> Rule R.23: Use `make_unique()` to make `unique_ptr`s ### <a name="Rr-make_unique"></a> Rule R.23: Use `make_unique()` to make `unique_ptr`s
**Reason**: for convenience and consistency with `shared_ptr`. **Reason**: for convenience and consistency with `shared_ptr`.
@ -5814,7 +5818,7 @@ The `make_shared()` version mentions `X` only once, so it is usually shorter (as
**Enforcement**: (Simple) Warn if a `Shared_ptr` is constructed from the result of `new` rather than `make_unique`. **Enforcement**: (Simple) Warn if a `Shared_ptr` is constructed from the result of `new` rather than `make_unique`.
### <a name ="Rr-weak_ptr"></a> R.24: Use `std::weak_ptr` to break cycles of `shared_ptr`s ### <a name="Rr-weak_ptr"></a> R.24: Use `std::weak_ptr` to break cycles of `shared_ptr`s
**Reason**: `shared_ptr`'s rely on use counting and the use count for a cyclic structure never goes to zero, so we need a mechanism to **Reason**: `shared_ptr`'s rely on use counting and the use count for a cyclic structure never goes to zero, so we need a mechanism to
be able to destroy a cyclic structure. be able to destroy a cyclic structure.
@ -6414,8 +6418,8 @@ or better using concepts
or or
double scalbn( // better: x*pow(FLT_RADIX, n); FLT_RADIX is usually 2 double scalbn( // better: x*pow(FLT_RADIX, n); FLT_RADIX is usually 2
double x; // base value double x, // base value
int n; // exponent int n // exponent
); );
or or
@ -6439,7 +6443,7 @@ or
auto s = v.size(); auto s = v.size();
auto h = t.future(); auto h = t.future();
auto q = new int[s]; auto q = new int[s];
auto f = [](int x){ return x+10; } auto f = [](int x){ return x+10; };
In each case, we save writing a longish, hard-to-remember type that the compiler already knows but a programmer could get wrong. In each case, we save writing a longish, hard-to-remember type that the compiler already knows but a programmer could get wrong.
@ -8066,7 +8070,7 @@ One strategy is to add a `valid()` operation to every resource handle:
void f() void f()
{ {
Vector<string> vs(100); // not std::vector: valid() added vector<string> vs(100); // not std::vector: valid() added
if (!vs.valid()) { if (!vs.valid()) {
// handle error or exit // handle error or exit
} }
@ -8606,7 +8610,7 @@ It also avoids brittle or inefficient workarounds. Convention: That's the way th
int sz; int sz;
}; };
Vector<double> v(10); vector<double> v(10);
v[7] = 9.9; v[7] = 9.9;
**Example, bad**: **Example, bad**:
@ -9441,14 +9445,14 @@ The two language mechanisms can be use effectively in combination, but a few des
// ... // ...
}; };
Vector<int> vi; vector<int> vi;
Vector<string> vs; vector<string> vs;
It is probably a dumb idea to define a `sort` as a member function of a container, It is probably a dumb idea to define a `sort` as a member function of a container,
but it is not unheard of and it makes a good example of what not to do. but it is not unheard of and it makes a good example of what not to do.
Given this, the compiler cannot know if `Vector<int>::sort()` is called, so it must generate code for it. Given this, the compiler cannot know if `vector<int>::sort()` is called, so it must generate code for it.
Similar for `Vector<string>::sort()`. Similar for `vector<string>::sort()`.
Unless those two functions are called that's code bloat. Unless those two functions are called that's code bloat.
Imagine what this would do to a class hierarchy with dozens of member functions and dozens of derived classes with many instantiations. Imagine what this would do to a class hierarchy with dozens of member functions and dozens of derived classes with many instantiations.
@ -10708,7 +10712,7 @@ Pointers should only refer to single objects, and pointer arithmetic is fragile
a[4] = 1; // OK a[4] = 1; // OK
a[count 1] = 2; // OK a[count - 1] = 2; // OK
use(a.data(), 3); // OK use(a.data(), 3); // OK
} }
@ -10798,8 +10802,8 @@ Issue a diagnostic for any indexing expression on an expression or variable of a
void f(int i, int j) void f(int i, int j)
{ {
a[i + j] = 12; // BAD, could be rewritten as... a[i + j] = 12; // BAD, could be rewritten as...
at(a, i + j) = 12; // OK - bounds-checked at(a, i + j) = 12; // OK - bounds-checked
} }
@ -11722,7 +11726,7 @@ When using exceptions as your error handling mechanism, always document this beh
## <a name ="Sd-consistent"></a> Define Copy, move, and destroy consistently ## <a name="Sd-consistent"></a> Define Copy, move, and destroy consistently
**Reason**: ??? **Reason**: ???
@ -11966,7 +11970,7 @@ Now `Named` has a default constructor, a destructor, and efficient copy and move
template<typename T> class Vector { template<typename T> class Vector {
public: public:
Vector<std::initializer_list<T>); vector<std::initializer_list<T>>;
// ... // ...
}; };