Adjust the spacing for consistency (#1626)

* Adjust the spacing for consistency

* Adjust the spacing for consistency-2
This commit is contained in:
Hyuk Myeong
2020-05-29 03:41:32 +09:00
committed by GitHub
parent 9960c7dd7f
commit 19f2672b9e

View File

@@ -2958,7 +2958,8 @@ It's efficient and eliminates bugs at the call site: `X&&` binds to rvalues, whi
##### Example ##### Example
void sink(vector<int>&& v) { // sink takes ownership of whatever the argument owned void sink(vector<int>&& v) // sink takes ownership of whatever the argument owned
{
// usually there might be const accesses of v here // usually there might be const accesses of v here
store_somewhere(std::move(v)); store_somewhere(std::move(v));
// usually no more use of v here; it is moved-from // usually no more use of v here; it is moved-from
@@ -2975,7 +2976,8 @@ Unique owner types that are move-only and cheap-to-move, such as `unique_ptr`, c
For example: For example:
template<class T> template<class T>
void sink(std::unique_ptr<T> p) { void sink(std::unique_ptr<T> p)
{
// use p ... possibly std::move(p) onward somewhere else // use p ... possibly std::move(p) onward somewhere else
} // p gets destroyed } // p gets destroyed
@@ -2996,7 +2998,8 @@ In that case, and only that case, make the parameter `TP&&` where `TP` is a temp
##### Example ##### Example
template<class F, class... Args> template<class F, class... Args>
inline auto invoke(F f, Args&&... args) { inline auto invoke(F f, Args&&... args)
{
return f(forward<Args>(args)...); return f(forward<Args>(args)...);
} }
@@ -3722,7 +3725,8 @@ This was primarily to avoid code of the form `(a = b) = c` -- such code is not c
{ {
public: public:
... ...
Foo& operator=(const Foo& rhs) { Foo& operator=(const Foo& rhs)
{
// Copy members. // Copy members.
... ...
return *this; return *this;
@@ -3914,7 +3918,8 @@ It's confusing. Writing `[=]` in a member function appears to capture by value,
int x = 0; int x = 0;
// ... // ...
void f() { void f()
{
int i = 0; int i = 0;
// ... // ...
@@ -3953,7 +3958,8 @@ This is fragile because it cannot generally be enforced to be safe in the langua
##### Example ##### Example
int sum(...) { int sum(...)
{
// ... // ...
while (/*...*/) while (/*...*/)
result += va_arg(list, int); // BAD, assumes it will be passed ints result += va_arg(list, int); // BAD, assumes it will be passed ints
@@ -3964,7 +3970,8 @@ This is fragile because it cannot generally be enforced to be safe in the langua
sum(3.14159, 2.71828); // BAD, undefined sum(3.14159, 2.71828); // BAD, undefined
template<class ...Args> template<class ...Args>
auto sum(Args... args) { // GOOD, and much more flexible auto sum(Args... args) // GOOD, and much more flexible
{
return (... + args); // note: C++17 "fold expression" return (... + args); // note: C++17 "fold expression"
} }
@@ -5642,7 +5649,8 @@ The return type of the factory should normally be `unique_ptr` by default; if so
class B { class B {
public: public:
B() { B()
{
/* ... */ /* ... */
f(); // BAD: C.82: Don't call virtual functions in constructors and destructors f(); // BAD: C.82: Don't call virtual functions in constructors and destructors
/* ... */ /* ... */
@@ -6173,7 +6181,8 @@ A *polymorphic class* is a class that defines or inherits at least one virtual f
// ... // ...
}; };
void f(B& b) { void f(B& b)
{
auto b2 = b; // oops, slices the object; b2.m() will return 'B' auto b2 = b; // oops, slices the object; b2.m() will return 'B'
} }
@@ -6196,7 +6205,8 @@ A *polymorphic class* is a class that defines or inherits at least one virtual f
// ... // ...
}; };
void f(B& b) { void f(B& b)
{
auto b2 = b; // ok, compiler will detect inadvertent copying, and protest auto b2 = b; // ok, compiler will detect inadvertent copying, and protest
} }
@@ -6572,14 +6582,12 @@ A type will provide a copy constructor and/or copy assignment operator to approp
##### Example, good ##### Example, good
struct base struct base {
{
virtual void update() = 0; virtual void update() = 0;
std::shared_ptr<int> sp; std::shared_ptr<int> sp;
}; };
struct derived : public base struct derived : public base {
{
void update() override {} void update() override {}
}; };
@@ -8015,7 +8023,8 @@ It also gives an opportunity to eliminate a separate allocation for the referenc
##### Example ##### Example
void test() { void test()
{
// OK: but repetitive; and separate allocations for the Bar and shared_ptr's use count // OK: but repetitive; and separate allocations for the Bar and shared_ptr's use count
shared_ptr<Bar> p {new Bar{7}}; shared_ptr<Bar> p {new Bar{7}};
@@ -9582,8 +9591,7 @@ be able to destroy a cyclic structure.
class bar; class bar;
class foo class foo {
{
public: public:
explicit foo(const std::shared_ptr<bar>& forward_reference) explicit foo(const std::shared_ptr<bar>& forward_reference)
: forward_reference_(forward_reference) : forward_reference_(forward_reference)
@@ -9592,8 +9600,7 @@ be able to destroy a cyclic structure.
std::shared_ptr<bar> forward_reference_; std::shared_ptr<bar> forward_reference_;
}; };
class bar class bar {
{
public: public:
explicit bar(const std::weak_ptr<foo>& back_reference) explicit bar(const std::weak_ptr<foo>& back_reference)
: back_reference_(back_reference) : back_reference_(back_reference)
@@ -10877,10 +10884,10 @@ 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. 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() { void write_to_file()
std::string buffer; // to avoid reallocations on every loop iteration
for (auto& o : objects)
{ {
std::string buffer; // to avoid reallocations on every loop iteration
for (auto& o : objects) {
// First part of the work. // First part of the work.
generate_first_string(buffer, o); generate_first_string(buffer, o);
write_to_file(buffer); write_to_file(buffer);
@@ -11804,11 +11811,13 @@ Sometimes, you may be tempted to resort to `const_cast` to avoid code duplicatio
class Foo { class Foo {
public: public:
// BAD, duplicates logic // BAD, duplicates logic
Bar& get_bar() { Bar& get_bar()
{
/* complex logic around getting a non-const reference to my_bar */ /* complex logic around getting a non-const reference to my_bar */
} }
const Bar& get_bar() const { const Bar& get_bar() const
{
/* same complex logic around getting a const reference to my_bar */ /* same complex logic around getting a const reference to my_bar */
} }
private: private:
@@ -11820,10 +11829,12 @@ Instead, prefer to share implementations. Normally, you can just have the non-`c
class Foo { class Foo {
public: public:
// not great, non-const calls const version but resorts to const_cast // not great, non-const calls const version but resorts to const_cast
Bar& get_bar() { Bar& get_bar()
{
return const_cast<Bar&>(static_cast<const Foo&>(*this).get_bar()); return const_cast<Bar&>(static_cast<const Foo&>(*this).get_bar());
} }
const Bar& get_bar() const { const Bar& get_bar() const
{
/* the complex logic around getting a const reference to my_bar */ /* the complex logic around getting a const reference to my_bar */
} }
private: private:
@@ -12002,7 +12013,8 @@ Explicit `move` is needed to explicitly move an object to another scope, notably
Usually, a `std::move()` is used as an argument to a `&&` parameter. Usually, a `std::move()` is used as an argument to a `&&` parameter.
And after you do that, assume the object has been moved from (see [C.64](#Rc-move-semantic)) and don't read its state again until you first set it to a new value. And after you do that, assume the object has been moved from (see [C.64](#Rc-move-semantic)) and don't read its state again until you first set it to a new value.
void f() { void f()
{
string s1 = "supercalifragilisticexpialidocious"; string s1 = "supercalifragilisticexpialidocious";
string s2 = s1; // ok, takes a copy string s2 = s1; // ok, takes a copy
@@ -12019,7 +12031,8 @@ And after you do that, assume the object has been moved from (see [C.64](#Rc-mov
void sink(unique_ptr<widget> p); // pass ownership of p to sink() void sink(unique_ptr<widget> p); // pass ownership of p to sink()
void f() { void f()
{
auto w = make_unique<widget>(); auto w = make_unique<widget>();
// ... // ...
sink(std::move(w)); // ok, give to sink() sink(std::move(w)); // ok, give to sink()
@@ -12039,7 +12052,8 @@ Never write `std::move()` on a const object, it is silently transformed into a c
##### Example, bad ##### Example, bad
vector<int> make_vector() { vector<int> make_vector()
{
vector<int> result; vector<int> result;
// ... load result with data // ... load result with data
return std::move(result); // bad; just write "return result;" return std::move(result); // bad; just write "return result;"
@@ -12058,14 +12072,16 @@ The language already knows that a returned value is a temporary object that can
##### Example ##### Example
void mover(X&& x) { void mover(X&& x)
{
call_something(std::move(x)); // ok call_something(std::move(x)); // ok
call_something(std::forward<X>(x)); // bad, don't std::forward an rvalue reference call_something(std::forward<X>(x)); // bad, don't std::forward an rvalue reference
call_something(x); // suspicious, why not std::move? call_something(x); // suspicious, why not std::move?
} }
template<class T> template<class T>
void forwarder(T&& t) { void forwarder(T&& t)
{
call_something(std::move(t)); // bad, don't std::move a forwarding reference call_something(std::move(t)); // bad, don't std::move a forwarding reference
call_something(std::forward<T>(t)); // ok call_something(std::forward<T>(t)); // ok
call_something(t); // suspicious, why not std::forward? call_something(t); // suspicious, why not std::forward?
@@ -12174,7 +12190,8 @@ In the rare cases where the slicing was deliberate the code can be surprising.
Shape s {c}; // copy construct only the Shape part of Circle Shape s {c}; // copy construct only the Shape part of Circle
s = c; // or copy assign only the Shape part of Circle s = c; // or copy assign only the Shape part of Circle
void assign(const Shape& src, Shape& dest) { void assign(const Shape& src, Shape& dest)
{
dest = src; dest = src;
} }
Circle c2 {{1, 1}, 43}; Circle c2 {{1, 1}, 43};
@@ -12712,7 +12729,8 @@ consider `gsl::finally()` as a cleaner and more reliable alternative to `goto ex
Often, a loop that requires a `break` is a good candidate for a function (algorithm), in which case the `break` becomes a `return`. Often, a loop that requires a `break` is a good candidate for a function (algorithm), in which case the `break` becomes a `return`.
//Original code: break inside loop //Original code: break inside loop
void use1(){ void use1()
{
std::vector<T> vec = {/* initialized with some values */}; std::vector<T> vec = {/* initialized with some values */};
T value; T value;
for (const T item : vec) { for (const T item : vec) {
@@ -12725,14 +12743,16 @@ Often, a loop that requires a `break` is a good candidate for a function (algori
} }
//BETTER: create a function and return inside loop //BETTER: create a function and return inside loop
T search(const std::vector<T> &vec) { T search(const std::vector<T> &vec)
{
for (const T &item : vec) { for (const T &item : vec) {
if (/* some condition*/) return item; if (/* some condition*/) return item;
} }
return T(); //default value return T(); //default value
} }
void use2() { void use2()
{
std::vector<T> vec = {/* initialized with some values */}; std::vector<T> vec = {/* initialized with some values */};
T value = search(vec); T value = search(vec);
/* then do something with value */ /* then do something with value */
@@ -13251,20 +13271,23 @@ This also applies to `%`.
##### Example, bad ##### Example, bad
double divide(int a, int b) { double divide(int a, int b)
{
// BAD, should be checked (e.g., in a precondition) // BAD, should be checked (e.g., in a precondition)
return a / b; return a / b;
} }
##### Example, good ##### Example, good
double divide(int a, int b) { double divide(int a, int b)
{
// good, address via precondition (and replace with contracts once C++ gets them) // good, address via precondition (and replace with contracts once C++ gets them)
Expects(b != 0); Expects(b != 0);
return a / b; return a / b;
} }
double divide(int a, int b) { double divide(int a, int b)
{
// good, address via check // good, address via check
return b ? a / b : quiet_NaN<double>(); return b ? a / b : quiet_NaN<double>();
} }
@@ -13500,8 +13523,7 @@ Simple code can be very fast. Optimizers sometimes do marvels with simple code
vector<uint8_t> v(100000); vector<uint8_t> v(100000);
for (size_t i = 0; i < v.size(); i += sizeof(uint64_t)) for (size_t i = 0; i < v.size(); i += sizeof(uint64_t)) {
{
uint64_t& quad_word = *reinterpret_cast<uint64_t*>(&v[i]); uint64_t& quad_word = *reinterpret_cast<uint64_t*>(&v[i]);
quad_word = ~quad_word; quad_word = ~quad_word;
} }
@@ -13975,7 +13997,8 @@ For further information of how to use synchronization well to eliminate data rac
There are many examples of data races that exist, some of which are running in There are many examples of data races that exist, some of which are running in
production software at this very moment. One very simple example: production software at this very moment. One very simple example:
int get_id() { int get_id()
{
static int id = 1; static int id = 1;
return id++; return id++;
} }
@@ -14101,7 +14124,8 @@ Application concepts are easier to reason about.
##### Example ##### Example
void some_fun() { void some_fun()
{
std::string msg, msg2; std::string msg, msg2;
std::thread publisher([&] { msg = "Hello"; }); // bad: less expressive std::thread publisher([&] { msg = "Hello"; }); // bad: less expressive
// and more error-prone // and more error-prone
@@ -14974,14 +14998,12 @@ There is no explicit locking and both correct (value) return and error (exceptio
void async_example() void async_example()
{ {
try try {
{
auto v1 = std::async(std::launch::async, read_value, "v1.txt"); auto v1 = std::async(std::launch::async, read_value, "v1.txt");
auto v2 = std::async(std::launch::async, read_value, "v2.txt"); auto v2 = std::async(std::launch::async, read_value, "v2.txt");
std::cout << v1.get() + v2.get() << '\n'; std::cout << v1.get() + v2.get() << '\n';
} }
catch (std::ios_base::failure & fail) catch (std::ios_base::failure & fail) {
{
// handle exception here // handle exception here
} }
} }
@@ -15251,7 +15273,8 @@ Sometimes C++ code allocates the `volatile` memory and shares it with "elsewhere
`volatile` local variables are nearly always wrong -- how can they be shared with other languages or hardware if they're ephemeral? `volatile` local variables are nearly always wrong -- how can they be shared with other languages or hardware if they're ephemeral?
The same applies almost as strongly to member variables, for the same reason. The same applies almost as strongly to member variables, for the same reason.
void f() { void f()
{
volatile int i = 0; // bad, volatile local variable volatile int i = 0; // bad, volatile local variable
// etc. // etc.
} }
@@ -15411,7 +15434,8 @@ C++ implementations tend to be optimized based on the assumption that exceptions
try { try {
for (gsl::index i = 0; i < vec.size(); ++i) for (gsl::index i = 0; i < vec.size(); ++i)
if (vec[i] == x) throw i; // found x if (vec[i] == x) throw i; // found x
} catch (int i) { }
catch (int i) {
return i; return i;
} }
return -1; // not found return -1; // not found
@@ -16359,7 +16383,8 @@ This gives a more precise statement of design intent, better readability, more e
// ... // ...
}; };
void f(const Point& pt) { void f(const Point& pt)
{
int x = pt.getx(); // ERROR, doesn't compile because getx was not marked const int x = pt.getx(); // ERROR, doesn't compile because getx was not marked const
} }
@@ -17731,7 +17756,8 @@ Eases tool creation.
} }
template<typename Iter> template<typename Iter>
Iter algo(Iter first, Iter last) { Iter algo(Iter first, Iter last)
{
for (; first != last; ++first) { for (; first != last; ++first) {
auto x = sqrt(*first); // potentially surprising dependency: which sqrt()? auto x = sqrt(*first); // potentially surprising dependency: which sqrt()?
helper(first, x); // potentially surprising dependency: helper(first, x); // potentially surprising dependency:
@@ -19017,7 +19043,8 @@ Doing so takes away an `#include`r's ability to effectively disambiguate and to
bool copy(/*... some parameters ...*/); // some function that happens to be named copy bool copy(/*... some parameters ...*/); // some function that happens to be named copy
int main() { int main()
{
copy(/*...*/); // now overloads local ::copy and std::copy, could be ambiguous copy(/*...*/); // now overloads local ::copy and std::copy, could be ambiguous
} }
@@ -21722,7 +21749,8 @@ Here is an example of the last option:
class B { class B {
public: public:
B() { B()
{
/* ... */ /* ... */
f(); // BAD: C.82: Don't call virtual functions in constructors and destructors f(); // BAD: C.82: Don't call virtual functions in constructors and destructors
/* ... */ /* ... */
@@ -21935,7 +21963,8 @@ Consider the following advice and requirements found in the C++ Standard:
Deallocation functions, including specifically overloaded `operator delete` and `operator delete[]`, fall into the same category, because they too are used during cleanup in general, and during exception handling in particular, to back out of partial work that needs to be undone. Deallocation functions, including specifically overloaded `operator delete` and `operator delete[]`, fall into the same category, because they too are used during cleanup in general, and during exception handling in particular, to back out of partial work that needs to be undone.
Besides destructors and deallocation functions, common error-safety techniques rely also on `swap` operations never failing -- in this case, not because they are used to implement a guaranteed rollback, but because they are used to implement a guaranteed commit. For example, here is an idiomatic implementation of `operator=` for a type `T` that performs copy construction followed by a call to a no-fail `swap`: Besides destructors and deallocation functions, common error-safety techniques rely also on `swap` operations never failing -- in this case, not because they are used to implement a guaranteed rollback, but because they are used to implement a guaranteed commit. For example, here is an idiomatic implementation of `operator=` for a type `T` that performs copy construction followed by a call to a no-fail `swap`:
T& T::operator=(const T& other) { T& T::operator=(const T& other)
{
auto temp = other; auto temp = other;
swap(temp); swap(temp);
return *this; return *this;