diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 9375153..97a1109 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -7768,7 +7768,7 @@ Arithmetic rules: ##### Reason Code using a library can be much easier to write than code working directly with language features, much shorter, tend to be of a higher level of abstraction, and the library code is presumably already tested. -The ISO C++ standard library is among the most widely know and best tested libraries. +The ISO C++ standard library is among the most widely known and best tested libraries. It is available as part of all C++ Implementations. ##### Example @@ -7869,27 +7869,25 @@ Readability. Minimize resource retention. Avoid accidental misuse of value. // ... 200 lines of code without intended use of fn or is ... } -This function is by most measure too long anyway, but the point is that the used by `fn` and the file handle held by `is` +This function is by most measure too long anyway, but the point is that the resources used by `fn` and the file handle held by `is` are retained for much longer than needed and that unanticipated use of `is` and `fn` could happen later in the function. In this case, it might be a good idea to factor out the read: - void fill_record(Record& r, const string& name) + Record load_record(const string& name) { string fn = name+".txt"; ifstream is {fn}; Record r; is >> r; + return r; } void use(const string& name) { - Record r; - fill_record(r, name); + Record r = load_record(name); // ... 200 lines of code ... } -I am assuming that `Record` is large and doesn't have a good move operation so that an out-parameter is preferable to returning a `Record`. - ##### Enforcement * Flag loop variable declared outside a loop and not used after the loop @@ -7982,7 +7980,7 @@ Here, there is a chance that the reader knows what `trim_tail` means and that th Argument names of large functions are de facto non-local and should be meaningful: - void complicated_algorithm(vector&vr, const vector& vi, map& out) + void complicated_algorithm(vector& vr, const vector& vi, map& out) // read from events in vr (marking used Records) for the indices in vi placing (name, index) pairs into out { // ... 500 lines of code using vr, vi, and out ... @@ -8056,7 +8054,9 @@ Flag all uses of ALL CAPS. For older code, accept ALL CAPS for macro names and f ##### Reason -One-declaration-per line increases readability and avoid mistake related to the C/C++ grammar. It leaves room for a `//`-comment. +One-declaration-per line increases readability and avoids mistakes related to +the C/C++ grammar. It also leaves room for a more descriptive end-of-line +comment. ##### Example, bad @@ -8107,7 +8107,7 @@ Consider: auto p = v.begin(); // vector::iterator auto s = v.size(); auto h = t.future(); - auto q = new int[s]; + auto q = make_unique(s); 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. @@ -8121,7 +8121,7 @@ In each case, we save writing a longish, hard-to-remember type that the compiler ##### Example - auto lst = { 1, 2, 3 }; // lst is an initializer list (obviously) + auto lst = { 1, 2, 3 }; // lst is an initializer list auto x{1}; // x is an int (after correction of the C++14 standard; initializer_list in C++11) ##### Note @@ -8269,7 +8269,8 @@ A good optimizer should know about input operations and eliminate the redundant ##### Example -Using an `uninitialized` value is a symptom of a problem and not a solution: +Using an `uninitialized` or sentinel value is a symptom of a problem and not a +solution: widget i = uninit; // bad widget j = uninit; @@ -8287,7 +8288,7 @@ Using an `uninitialized` value is a symptom of a problem and not a solution: j = f4(); } -Now the compiler cannot even simply detect a used-before-set. +Now the compiler cannot even simply detect a used-before-set. Further, we've introduced complexity in the state space for widget: which operations are valid on an `unint` widget and which are not? ##### Note @@ -8315,7 +8316,7 @@ or maybe: * Flag every uninitialized variable. Don't flag variables of user-defined types with default constructors. * Check that an uninitialized buffer is written into *immediately* after declaration. - Passing a uninitialized variable as a reference to non-`const` argument can be assumed to be a write into the variable. + Passing an uninitialized variable as a reference to non-`const` argument can be assumed to be a write into the variable. ### ES.21: Don't introduce a variable (or constant) before you need to use it @@ -8331,7 +8332,7 @@ Readability. To limit the scope in which the variable can be used. ##### Enforcement -Flag declaration that distant from their first use. +Flag declarations that are distant from their first use. ### ES.22: Don't declare a variable until you have a value to initialize it with @@ -8452,11 +8453,13 @@ Tricky. * Don't flag uses of `=` for simple initializers. * Look for `=` after `auto` has been seen. -### ES.24: Use a `unique_ptr` to hold pointers in code that may throw +### ES.24: Use a `unique_ptr` to hold pointers ##### Reason -Using `std::unique_ptr` is the simplest way to avoid leaks. And it is free compared to alternatives +Using `std::unique_ptr` is the simplest way to avoid leaks. It is reliable, it +makes the type system do much of the work to validate ownership safety, it +increases readability, and it has zero or near zero runtime cost. ##### Example @@ -8475,7 +8478,7 @@ If `leak == true` the object pointed to by `p2` is leaked and the object pointed Look for raw pointers that are targets of `new`, `malloc()`, or functions that may return such pointers. -### ES.25: Declare an objects `const` or `constexpr` unless you want to modify its value later on +### ES.25: Declare objects `const` or `constexpr` unless you want to modify its value later on ##### Reason @@ -8492,7 +8495,9 @@ That way you can't change the value by mistake. That way may offer the compiler ##### Enforcement -Look to see if a variable is actually mutated, and flag it if not. Unfortunately, it may be impossible to detect when a non-`const` was not intended to vary. +Look to see if a variable is actually mutated, and flag it if +not. Unfortunately, it may be impossible to detect when a non-`const` was not +*intended* to vary (vs when it merely did not vary). ### ES.26: Don't use a variable for two unrelated purposes @@ -8721,7 +8726,7 @@ Statements control the flow of control (except for function calls and exception * Readability. * Efficiency: A `switch` compares against constants and is usually better optimized than a series of tests in an `if`-`then`-`else` chain. -* a `switch` is enables some heuristic consistency checking. For example, has all values of an `enum` been covered? If not, is there a `default`? +* a `switch` is enables some heuristic consistency checking. For example, have all values of an `enum` been covered? If not, is there a `default`? ##### Example @@ -8768,7 +8773,7 @@ Readability. Error prevention. Efficiency. cout << v[i] + v[i-1] << '\n'; for (int i = 0; i < v.size(); ++i) // possible side-effect: can't be a range-for - cout << f(&v[i]) << '\n'; + cout << f(v, &v[i]) << '\n'; for (int i = 0; i < v.size(); ++i) { // body messes with loop variable: can't be a range-for if (i % 2) @@ -8777,7 +8782,7 @@ Readability. Error prevention. Efficiency. cout << v[i] << '\n'; } -A human or a good static analyzer may determine that there really isn't a side effect on `v` in `f(&v[i])` so that the loop can be rewritten. +A human or a good static analyzer may determine that there really isn't a side effect on `v` in `f(v, &v[i])` so that the loop can be rewritten. "Messing with the loop variable" in the body of a loop is typically best avoided. @@ -8787,13 +8792,17 @@ Don't use expensive copies of the loop variable of a range-`for` loop: for (string s : vs) // ... -This will copy each elements of `vs` into `s`. Better +This will copy each elements of `vs` into `s`. Better: for (string& s : vs) // ... +Better still, if the loop variable isn't modified or copied: + + for (const string& s : vs) // ... + ##### Enforcement -Look at loops, if a traditional loop just looks at each element of a sequence, and there are no side-effects on what it does with the elements, rewrite the loop to a for loop. +Look at loops, if a traditional loop just looks at each element of a sequence, and there are no side-effects on what it does with the elements, rewrite the loop to a ranged-for loop. ### ES.72: Prefer a `for`-statement to a `while`-statement when there is an obvious loop variable @@ -8868,7 +8877,7 @@ is only accessible in the loop body unblocks optimizations such as hoisting, str ##### Reason Readability, avoidance of errors. -The termination conditions is at the end (where it can be overlooked) and the condition is not checked the first time through. ??? +The termination condition is at the end (where it can be overlooked) and the condition is not checked the first time through. ??? ##### Example @@ -9008,15 +9017,16 @@ Readability. ##### Example for (i = 0; i < max; ++i); // BAD: the empty statement is easily overlooked - v[i] = f(v[i]); + v[i] = f(v[i]); for (auto x : v) { // better // nothing } - + v[i] = f(v[i]); + ##### Enforcement -Flag empty statements that are not blocks and don't "contain" comments. +Flag empty statements that are not blocks and don't contain comments. ### ES.86: Avoid modifying loop control variables inside the body of raw for-loops @@ -9037,6 +9047,14 @@ The loop control up front should enable correct reasoning about what is happenin // } + bool skip=false; + for (int i=0; i<10; ++i) { + if (skip) { skip = false; continue; } + // + if (/* something */) skip = true; // Better: using two variable for two concepts. + // + } + ##### Enforcement Flag variables that are potentially updated (have a non-const use) in both the loop control iteration-expression and the loop body. @@ -9213,9 +9231,9 @@ Unnamed constants embedded in expressions are easily overlooked and often hard t No, we don't all know that there are 12 months, numbered 1..12, in a year. Better: - constexpr int last_month = 12; // months are numbered 1..12 + constexpr int month_count = 12; // months are numbered 1..12 - for (int m = first_month; m <= last_month; ++m) // better + for (int m = first_month; m <= month_count; ++m) // better cout << month[m] << '\n'; Better still, don't expose constants: @@ -9276,7 +9294,10 @@ A good analyzer can detect all narrowing conversions. However, flagging all narr ##### Reason -Readability. Minimize surprises: `nullptr` cannot be confused with an `int`. +Readability. Minimize surprises: `nullptr` cannot be confused with an +`int`. `nullptr` also has a well-specified (very restrictive) type, and thus +works in more scenarios where type deduction might do the wrong thing on `NULL` +or `0`. ##### Example @@ -9310,9 +9331,9 @@ If there is not, maybe there ought to be, rather than applying a local fix (cast ##### Note -Casts are necessary in a systems programming language. -For example, how else would we get the address of a device register into a pointer. -However, casts are seriously overused as well as a major source of errors. +Casts are necessary in a systems programming language. For example, how else +would we get the address of a device register into a pointer? However, casts +are seriously overused as well as a major source of errors. ##### Note @@ -9363,7 +9384,7 @@ It makes a lie out of `const`. ##### Note Usually the reason to "cast away `const`" is to allow the updating of some transient information of an otherwise immutable object. -Examples are cashing, memorization, and precomputation. +Examples are caching, memoization, and precomputation. Such examples are often handled as well or better using `mutable` or an indirection than with a `const_cast`. ##### Example @@ -9378,7 +9399,7 @@ Flag `const_cast`s. ##### Reason -Constructs that cannot overflow, don't, and usually run faster: +Constructs that cannot overflow do not overflow (and usually run faster): ##### Example @@ -9607,7 +9628,9 @@ Unsigned types support bit manipulation without surprises from sign bits. ??? -**Exception**: Use signed types if you really want modulo arithmetic. +**Exception**: Use unsigned types if you really want modulo arithmetic - add +comments as necessary noting the reliance on overflow behavior, as such code +is going to be surprising for many programmers. ##### Enforcement @@ -9623,7 +9646,9 @@ Signed types support modulo arithmetic without surprises from lack of sign bits. ??? -**Exception**: Use unsigned types if you really want bit manipulation. +**Exception**: Use unsigned types if you really want modulo arithmetic - add +comments as necessary noting the reliance on overflow behavior, as such code +is going to be surprising for many programmers. ##### Enforcement