From 9cc50836bd62d39900b3f9784d88a1589390c03b Mon Sep 17 00:00:00 2001 From: Thibault Kruse Date: Fri, 26 May 2017 13:17:12 +0900 Subject: [PATCH] Diverse style fixes Remove whitespace at end of line fix code indent whitepsace between operators align comments add method name to fix syntax highlighting typo: start with capital letter Apply snake-case naming for unctions (for consistency) --- CppCoreGuidelines.md | 173 ++++++++++++++++++++++--------------------- 1 file changed, 87 insertions(+), 86 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 7bee82a..472f290 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -853,7 +853,7 @@ We could check earlier and improve the code: // ... } -Now, `m<=n` can be checked at the point of call (early) rather than later. +Now, `m <= n` can be checked at the point of call (early) rather than later. If all we had was a typo so that we meant to use `n` as the bound, the code could be further simplified (eliminating the possibility of an error): void use3(int m) @@ -1874,7 +1874,7 @@ Note: `length()` is, of course, `std::strlen()` in disguise. Consider: - void copy_n(const T* p, T* q, int n); // copy from [p:p+n) to [q:q+n) + void copy_n(const T* p, T* q, int n); // copy from [p:p + n) to [q:q + n) What if there are fewer than `n` elements in the array pointed to by `q`? Then, we overwrite some probably unrelated memory. What if there are fewer than `n` elements in the array pointed to by `p`? Then, we read some probably unrelated memory. @@ -1964,13 +1964,14 @@ Having many arguments opens opportunities for confusion. Passing lots of argumen The two most common reasons why functions have too many parameters are: - 1. *Missing an abstraction.* There is an abstraction missing, so that a compound value is being - passed as individual elements instead of as a single object that enforces an invariant. - This not only expands the parameter list, but it leads to errors because the component values - are no longer protected by an enforced invariant. +1. *Missing an abstraction.* + There is an abstraction missing, so that a compound value is being + passed as individual elements instead of as a single object that enforces an invariant. + This not only expands the parameter list, but it leads to errors because the component values + are no longer protected by an enforced invariant. - 2. *Violating "one function, one responsibility."* The function is trying to do more than one - job and should probably be refactored. +2. *Violating "one function, one responsibility."* + The function is trying to do more than one job and should probably be refactored. ##### Example @@ -2040,13 +2041,13 @@ Adjacent arguments of the same type are easily swapped by mistake. Consider: - void copy_n(T* p, T* q, int n); // copy from [p:p+n) to [q:q+n) + void copy_n(T* p, T* q, int n); // copy from [p:p + n) to [q:q + n) This is a nasty variant of a K&R C-style interface. It is easy to reverse the "to" and "from" arguments. Use `const` for the "from" argument: - void copy_n(const T* p, T* q, int n); // copy from [p:p+n) to [q:q+n) + void copy_n(const T* p, T* q, int n); // copy from [p:p + n) to [q:q + n) ##### Exception @@ -2389,8 +2390,8 @@ Functions with complex control structures are more likely to be long and more li Consider: - double simpleFunc(double val, int flag1, int flag2) - // simpleFunc: takes a value and calculates the expected ASIC output, + double simple_func(double val, int flag1, int flag2) + // simple_func: takes a value and calculates the expected ASIC output, // given the two mode flags. { double intermediate; @@ -2433,8 +2434,8 @@ We can refactor: // ??? } - double simpleFunc(double val, int flag1, int flag2) - // simpleFunc: takes a value and calculates the expected ASIC output, + double simple_func(double val, int flag1, int flag2) + // simple_func: takes a value and calculates the expected ASIC output, // given the two mode flags. { if (flag1 > 0) @@ -2527,7 +2528,7 @@ Most computation is best done at run time. Any API that may eventually depend on high-level runtime configuration or business logic should not be made `constexpr`. Such customization can not be -evaluated by the compiler, and any `constexpr` functions that depended upon +evaluated by the compiler, and any `constexpr` functions that depended upon that API would have to be refactored or drop `constexpr`. ##### Enforcement @@ -3191,7 +3192,7 @@ Informal/non-explicit ranges are a source of errors. ##### Note Ranges are extremely common in C++ code. Typically, they are implicit and their correct use is very hard to ensure. -In particular, given a pair of arguments `(p, n)` designating an array \[`p`:`p+n`), +In particular, given a pair of arguments `(p, n)` designating an array \[`p`:`p + n`), it is in general impossible to know if there really are `n` elements to access following `*p`. `span` and `span_p` are simple helper classes designating a \[`p`:`q`) range and a range starting with `p` and ending with the first element for which a predicate is true, respectively. @@ -4186,7 +4187,7 @@ For example, a derived class might be allowed to skip a run-time check because i int mem(int x, int y) { /* ... do something ... */ - return do_bar(x+y); // OK: derived class can bypass check + return do_bar(x + y); // OK: derived class can bypass check } } @@ -6969,15 +6970,15 @@ This kind of "vector" isn't meant to be used as a base class at all. ##### Example, bad - class Shape { - public: + class Shape { + public: // ... interface functions ... - protected: + protected: // data for use in derived classes: Color fill_color; Color edge_color; Style st; - }; + }; Now it is up to every derived `Shape` to manipulate the protected data correctly. This has been popular, but also a major source of maintenance problems. @@ -7052,7 +7053,7 @@ Especially to break apart monolithic interfaces into "aspects" of behavior suppo }; `istream` provides the interface to input operations; `ostream` provides the interface to output operations. -`iostream` provides the union of the `istream` and `ostream` interfaces and the synchronization needed to allow both on a single stream. +`iostream` provides the union of the `istream` and `ostream` interfaces and the synchronization needed to allow both on a single stream. ##### Note @@ -7081,7 +7082,7 @@ If the operations are virtual the use of inheritance is necessary, if not using }; `istream` provides the interface to input operations (and some data); `ostream` provides the interface to output operations (and some data). -`iostream` provides the union of the `istream` and `ostream` interfaces and the synchronization needed to allow both on a single stream. +`iostream` provides the union of the `istream` and `ostream` interfaces and the synchronization needed to allow both on a single stream. ##### Note @@ -7090,13 +7091,13 @@ This a relatively rare use because implementation can often be organized into a ##### Example Sometimes, an "implementation attribute" is more like a "mixin" that determine the behavior of an implementation and inject -members to enable the implementation of the policies it requires. +members to enable the implementation of the policies it requires. For example, see `std::enable_shared_from_this` or various bases from boost.intrusive (e.g. `list_base_hook` or `intrusive_ref_counter`). ##### Enforcement -??? +??? ### C.137: Use `virtual` bases to avoid overly general base classes @@ -7167,7 +7168,7 @@ Without a using declaration, member functions in the derived class hide the enti }; class D: public B { public: - int f(int i) override { std::cout << "f(int): "; return i+1; } + int f(int i) override { std::cout << "f(int): "; return i + 1; } }; int main() { @@ -7180,7 +7181,7 @@ Without a using declaration, member functions in the derived class hide the enti class D: public B { public: - int f(int i) override { std::cout << "f(int): "; return i+1; } + int f(int i) override { std::cout << "f(int): "; return i + 1; } using B::f; // exposes f(double) }; @@ -9871,7 +9872,7 @@ When concepts become available, we can (and should) be more specific about the t ForwardIterator p = algo(x, y, z); ##### Example (C++17) - + auto [ quotient, remainder ] = div(123456, 73); // break out the members of the div_t result ##### Enforcement @@ -10384,17 +10385,17 @@ Readability and safety. As an optimization, you may want to reuse a buffer as a scratch pad, but even then prefer to limit the variable'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. - { + void write_to_file() { std::string buffer; // to avoid reallocations on every loop iteration for (auto& o : objects) { // First part of the work. - generateFirstString(buffer, o); - writeToFile(buffer); + generate_first_String(buffer, o); + write_to_file(buffer); // Second part of the work. - generateSecondString(buffer, o); - writeToFile(buffer); + generate_second_string(buffer, o); + write_to_file(buffer); // etc... } @@ -11060,11 +11061,11 @@ There are exceedingly clever used of this "idiom", but they are far rarer than t ##### Note - Unnamed function arguments are fine. +Unnamed function arguments are fine. ##### Enforcement - Flag statements that are just a temporary +Flag statements that are just a temporary ### ES.85: Make empty statements visible @@ -11240,22 +11241,22 @@ Access into an array with known bounds using a constant as a subscript can be va { if (count < 2) return; - int* q = p + 1; // BAD + int* q = p + 1; // BAD ptrdiff_t d; int n; - d = (p - &n); // OK - d = (q - p); // OK + d = (p - &n); // OK + d = (q - p); // OK - int n = *p++; // BAD + int n = *p++; // BAD if (count < 6) return; - p[4] = 1; // BAD + p[4] = 1; // BAD - p[count - 1] = 2; // BAD + p[count - 1] = 2; // BAD - use(&p[0], 3); // BAD + use(&p[0], 3); // BAD } ##### Example, good @@ -11264,17 +11265,17 @@ Access into an array with known bounds using a constant as a subscript can be va { if (a.length() < 2) return; - int n = a[0]; // OK + int n = a[0]; // OK span q = a.subspan(1); // OK if (a.length() < 6) return; - 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 } ##### Note @@ -11589,12 +11590,12 @@ What would you think this fragment prints? The result is at best implementation 2 0 4611686018427387904 -Adding +Adding *q = 666; cout << d << ' ' << *p << ' ' << *q << '\n'; -I got +I got 3.29048e-321 666 666 @@ -12260,7 +12261,7 @@ can be surprising for many programmers. ##### Reason Because most arithmetic is assumed to be signed; -`x-y` yields a negative number when `y>x` except in the rare cases where you really want modulo arithmetic. +`x - y` yields a negative number when `y > x` except in the rare cases where you really want modulo arithmetic. ##### Example @@ -12270,23 +12271,23 @@ This is even more true for mixed signed and unsigned arithmetic. template T subtract(T x, T2 y) { - return x-y; + return x - y; } void test() { int s = 5; unsigned int us = 5; - cout << subtract(s, 7) << '\n'; // -2 - cout << subtract(us, 7u) << '\n'; // 4294967294 - cout << subtract(s, 7u) << '\n'; // -2 - cout << subtract(us, 7) << '\n'; // 4294967294 - cout << subtract(s, us+2) << '\n'; // -2 - cout << subtract(us, s+2) << '\n'; // 4294967294 + cout << subtract(s, 7) << '\n'; // -2 + cout << subtract(us, 7u) << '\n'; // 4294967294 + cout << subtract(s, 7u) << '\n'; // -2 + cout << subtract(us, 7) << '\n'; // 4294967294 + cout << subtract(s, us + 2) << '\n'; // -2 + cout << subtract(us, s + 2) << '\n'; // 4294967294 } Here we have been very explicit about what's happening, -but if you had seen `us-(s+2)` or `s+=2; ... us-s`, would you reliably have suspected that the result would print as `4294967294`? +but if you had seen `us - (s + 2)` or `s += 2; ...; us - s`, would you reliably have suspected that the result would print as `4294967294`? ##### Exception @@ -12301,10 +12302,10 @@ The build-in array uses signed types for subscripts. This makes surprises (and bugs) inevitable. int a[10]; - for (int i=0; i < 10; ++i) a[i]=i; + for (int i = 0; i < 10; ++i) a[i] = i; vector v(10); // compares signed to unsigned; some compilers warn - for (int i=0; v.size() < 10; ++i) v[i]=i; + for (int i = 0; v.size() < 10; ++i) v[i] = i; int a2[-2]; // error: negative size @@ -12491,13 +12492,13 @@ To enable better error detection. vector vec {1, 2, 3, 4, 5}; - for (int i=0; i < vec.size(); i+=2) // mix int and unsigned + for (int i = 0; i < vec.size(); i += 2) // mix int and unsigned cout << vec[i] << '\n'; - for (unsigned i=0; i < vec.size(); i+=2) // risk wraparound + for (unsigned i = 0; i < vec.size(); i += 2) // risk wraparound cout << vec[i] << '\n'; - for (vector::size_type i=0; i < vec.size(); i+=2) // verbose + for (vector::size_type i = 0; i < vec.size(); i += 2) // verbose cout << vec[i] << '\n'; - for (auto i=0; i < vec.size(); i+=2) // mix int and unsigned + for (auto i = 0; i < vec.size(); i += 2) // mix int and unsigned cout << vec[i] << '\n'; ##### Note @@ -13919,7 +13920,7 @@ Flag all unnamed `lock_guard`s and `unique_lock`s. ##### Reason -It should be obvious to a reader that the data is to be guarded and how. This decreases the chance of the wrong mutex being locked, or the mutex not being locked. +It should be obvious to a reader that the data is to be guarded and how. This decreases the chance of the wrong mutex being locked, or the mutex not being locked. Using a `synchronized_value` ensures that the data has a mutex, and the right mutex is locked when the data is accessed. See the [WG21 proposal](http://wg21.link/p0290)) to add `synchronized_value` to a future TS or revision of the C++ standard. @@ -15249,7 +15250,7 @@ Exception specifications make error handling brittle, impose a run-time cost, an // ... } -if `f()` throws an exception different from `X` and `Y` the unexpected handler is invoked, which by default terminates. +If `f()` throws an exception different from `X` and `Y` the unexpected handler is invoked, which by default terminates. That's OK, but say that we have checked that this cannot happen and `f` is changed to throw a new exception `Z`, we now have a crash on our hands unless we change `use()` (and re-test everything). The snag is that `f()` may be in a library we do not control and the new exception is not anything that `use()` can do @@ -17939,11 +17940,11 @@ The argument-type error for `bar` cannot be caught until link time because of th ##### Example - #include - #include - #include - #include - #include + #include + #include + #include + #include + #include using namespace std; @@ -17956,7 +17957,7 @@ could be distracting. The use of `using namespace std;` leaves the programmer open to a name clash with a name from the standard library - #include + #include using namespace std; int g(int x) @@ -17998,7 +17999,7 @@ Doing so takes away an `#include`r's ability to effectively disambiguate and to // user.cpp #include "bad.h" - + bool copy(/*... some parameters ...*/); // some function that happens to be named copy int main() { @@ -18093,7 +18094,7 @@ but it is not required to do so by transitively including the entire `` resulting in the popular beginner question "why doesn't `getline(cin,s);` work?" or even an occasional "`string`s cannot be compared with `==`). -The solution is to explicitly `#include`: +The solution is to explicitly `#include `: #include #include @@ -18412,7 +18413,7 @@ The important issue of non-ASCII character sets and encodings (e.g., `wchar_t`, See also [regular expressions](#SS-regex). Here, we use "sequence of characters" or "string" to refer to a sequence of characters meant to be read as text (somehow, eventually). -We don't consider +We don't consider String summary: @@ -18480,11 +18481,11 @@ Don't use C-style strings for operations that require non-trivial memory managem { int l1 = strlen(s1); int l2 = strlen(s2); - char* p = (char*)malloc(l1+l2+2); + char* p = (char*) malloc(l1 + l2 + 2); strcpy(p, s1, l1); p[l1] = '.'; - strcpy(p+l1+1, s2, l2); - p[l1+l2+1] = 0; + strcpy(p + l1 + 1, s2, l2); + p[l1 + l2 + 1] = 0; return res; } @@ -18595,7 +18596,7 @@ The array `arr` is not a C-style string because it is not zero-terminated. See [`zstring`](#Rstr-zstring), [`string`](#Rstr-string), and [`string_span`](#Rstr-view). ##### Enforcement - + * Flag uses of `[]` on a `char*` ### SL.str.5: Use `std::byte` to refer to byte values that do not necessarily represent characters @@ -18616,7 +18617,7 @@ C++17 ##### Enforcement ??? - + ### SL.str.10: Use `std::string` when you need to perform locale-sensitive string operations @@ -18689,7 +18690,7 @@ Iostream rule summary: * [SL.io.1: Use character-level input only when you have to](#Rio-low) * [SL.io.2: When reading, always consider ill-formed input](#Rio-validate) * [SL.io.3: Prefer iostreams for I/O](#Rio-streams) -* [SL.io.10: Unless you use `printf`-family functions call `ios_base::sync_with_stdio(false)`](#Rio-sync) +* [SL.io.10: Unless you use `printf`-family functions call `ios_base::sync_with_stdio(false)`](#Rio-sync) * [SL.io.50: Avoid `endl`](#Rio-endl) * [???](#???) @@ -19375,7 +19376,7 @@ Enabling a profile is implementation defined; typically, it is set in the analys To suppress enforcement of a profile check, place a `suppress` annotation on a language contract. For example: - [[suppress(bounds)]] char* raw_find(char* p, int n, char x) // find x in p[0]..p[n-1] + [[suppress(bounds)]] char* raw_find(char* p, int n, char x) // find x in p[0]..p[n - 1] { // ... } @@ -19630,7 +19631,7 @@ These types allow the user to distinguish between owning and non-owning pointers These "views" are never owners. -References are never owners. Note: References have many opportunities to outlive the objects they refer to (returning a local variable by reference, holding a reference to an element of a vector and doing `push_back`, binding to `std::max(x,y+1)`, etc. The Lifetime safety profile aims to address those things, but even so `owner` does not make sense and is discouraged. +References are never owners. Note: References have many opportunities to outlive the objects they refer to (returning a local variable by reference, holding a reference to an element of a vector and doing `push_back`, binding to `std::max(x, y + 1)`, etc. The Lifetime safety profile aims to address those things, but even so `owner` does not make sense and is discouraged. The names are mostly ISO standard-library style (lower case and underscore): @@ -19658,7 +19659,7 @@ If something is not supposed to be `nullptr`, say so: * `not_null` // `T` is usually a pointer type (e.g., `not_null` and `not_null>`) that may not be `nullptr`. `T` can be any type for which `==nullptr` is meaningful. -* `span` // `[`p`:`p+n`)`, constructor from `{p, q}` and `{p, n}`; `T` is the pointer type +* `span` // `[`p`:`p + n`)`, constructor from `{p, q}` and `{p, n}`; `T` is the pointer type * `span_p` // `{p, predicate}` \[`p`:`q`) where `q` is the first element for which `predicate(*p)` is true * `string_span` // `span` * `cstring_span` // `span` @@ -19695,7 +19696,7 @@ Use `not_null` for C-style strings that cannot be `nullptr`. ??? Do we These assertions are currently macros (yuck!) and must appear in function definitions (only) pending standard committee decisions on contracts and assertion syntax. See [the contract proposal](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0380r1.pdf); using the attribute syntax, -for example, `Expects(p!=nullptr)` will become `[[expects: p!=nullptr]]`. +for example, `Expects(p != nullptr)` will become `[[expects: p != nullptr]]`. ## GSL.util: Utilities @@ -21029,7 +21030,7 @@ When is a class a container? ??? A relatively informal definition of terms used in the guidelines (based of the glossary in [Programming: Principles and Practice using C++](http://www.stroustrup.com/programming.html)) -More information on many topics about C++ can be found on the [Standard C++ Foundation](https://isocpp.org)'s site. +More information on many topics about C++ can be found on the [Standard C++ Foundation](https://isocpp.org)'s site. * *ABI*: Application Binary Interface, a specification for a specific hardware platform combined with the operating system. Contrast with API. * *abstract class*: a class that cannot be directly used to create objects; often used to define an interface to derived classes. @@ -21157,7 +21158,7 @@ In particular, an object of a regular type can be copied and the result of a cop * *subtype*: derived type; a type that has all the properties of a type and possibly more. * *supertype*: base type; a type that has a subset of the properties of a type. * *system*: (1) a program or a set of programs for performing a task on a computer; (2) a shorthand for "operating system", that is, the fundamental execution environment and tools for a computer. -* *TS*: [Technical Specification](https://www.iso.org/deliverables-all.html?type=ts), A Technical Specification addresses work still under technical development, or where it is believed that there will be a future, but not immediate, possibility of agreement on an International Standard. A Technical Specification is published for immediate use, but it also provides a means to obtain feedback. The aim is that it will eventually be transformed and republished as an International Standard. +* *TS*: [Technical Specification](https://www.iso.org/deliverables-all.html?type=ts), A Technical Specification addresses work still under technical development, or where it is believed that there will be a future, but not immediate, possibility of agreement on an International Standard. A Technical Specification is published for immediate use, but it also provides a means to obtain feedback. The aim is that it will eventually be transformed and republished as an International Standard. * *template*: a class or a function parameterized by one or more types or (compile-time) values; the basic C++ language construct supporting generic programming. * *testing*: a systematic search for errors in a program. * *trade-off*: the result of balancing several design and implementation criteria.