Merge pull request #515 from abseil/axgillies-patch-1

Update 2018-04-20-totw-120.md

Author: axgillies

_posts/2018-04-20-totw-120.md

@@ -10,15 +10,14 @@ order: "120"
 
 Originally posted as TotW #120 on August 16, 2012
 
-*by Samuel Benzaquen, [(sbenza@google.com)](mailto:sbenza@gmail.com)*
+*by Samuel Benzaquen, [(sbenza@google.com)](mailto:sbenza@google.com)*
 
-Let's suppose you have a code snippet like below, which relies on an RAII
-cleanup function, which seems to be working as expected:
+Let's suppose you have this code snippet, which seems to be working as expected:
 
 ```c++
-MyStatus DoSomething() {
-  MyStatus status;
-  auto log_on_error = RunWhenOutOfScope([&status] {
+absl::Status DoSomething() {
+  absl::Status status;
+  auto log_on_error = absl::MakeCleanup([&status] {
     if (!status.ok()) LOG(ERROR) << status;
   });
   status = DoA();
@@ -31,45 +30,43 @@ MyStatus DoSomething() {
 }
 ```
 
-A refactor changes the last line from `return status;` to
-`return MyStatus();` and suddenly the code stopped logging errors.
+A refactor changes the last line from `return status;` to `return
+absl::OkStatus();` and suddenly the code stopped logging the errors.
 
 What is going on?
 
 ## Summary
 
-Never access (read or write) the return variable of a function after the return
-statement has run. Unless you take great care to do it correctly, the behavior
-is unspecified.
+Never access (read or write) the return variable after the return statement has
+run. Unless you take great care to do it correctly, the behavior is unspecified.
 
 Return variables are implicitly accessed by destructors after they have been
-copied or move (See Section 6.6.3 of the C++11 standard [stmt.return]), which
-is how this unexpected access occurs, but the copy/move may be elided, which
-is why behavior is undefined.
+copied or moved ([stmt.return]), which is how this unexpected access occurs, but
+the copy/move may be elided, which is why behavior is unspecified.
 
 This tip only applies when you are returning a non-reference local variable.
 Returning any other expression will not trigger this problem.
 
 ## The Problem
 
 There are 2 different optimizations on return statements that can modify the
-behavior of our original code snippet: NRVO (See [TotW #11](/tips/11)) and
+behavior of that code snippet: the named return value optimization (NRVO) and
 implicit moves.
 
-The *before* code worked because copy elision is taking place and the
-`return` statement is not actually doing any work. The variable `status` is
-already constructed in the return address and the cleanup object is seeing this
-unique instance of the `MyStatus` object _after_ the return statement.
+The *before* code was working because NRVO is being performed and the `return`
+statement is not actually doing any work. The variable `status` is already
+constructed in the return address and the cleanup object is seeing this unique
+instance of the `Status` object _after_ the return statement.
 
-In the *after* code, copy elision is not being applied and the returned variable
-is being moved into the return value. `RunWhenOutOfScope()` runs after the
-move operation is done and it is seeing a moved-from `MyStatus` object.
+In the *after* code, NRVO is not being performed and the returned variable is
+being moved into the return value. The cleanup object is being run after the
+move operation is done and it is seeing a moved-from `Status`.
 
-Note that the *before* code was not correct either, as it relies on the copy
-elision optimization for correctness. We encourage you to rely on copy elision
-for performance (See [TotW #24](/tips/24)), but not correctness. After all,
-copy elision is an _optional_ optimization and compiler options or quality of
-implementation of the compiler can affect whether or not it happens.
+Note that the *before* code was not correct either, as it relies on NRVO for
+correctness. We encourage you to rely on NRVO for performance (See TotW #24),
+but not correctness. After all, NRVO is an _optional_ optimization and compiler
+options or quality of implementation of the compiler can affect whether or not
+it happens.
 
 ## Solution
 
@@ -81,8 +78,8 @@ processing, and one that calls the first one and does the post-processing (ie
 logging on error). Eg:
 
 ```c++
-MyStatus DoSomething() {
-  MyStatus status;
+absl::Status DoSomething() {
+  absl::Status status;
   status = DoA();
   if (!status.ok()) return status;
   status = DoB();
@@ -91,26 +88,27 @@ MyStatus DoSomething() {
   if (!status.ok()) return status;
   return status;
 }
-MyStatus DoSomethingAndLog() {
-  MyStatus status = DoSomething();
+
+absl::Status DoSomethingAndLog() {
+  absl::Status status = DoSomething();
   if (!status.ok()) LOG(ERROR) << status;
   return status;
 }
 ```
 
 If you are only reading the value, you could also make sure to disable the
 optimizations instead. That would force a copy to be made all the time and the
-post-processing will not see a moved-from value. E.g.:
+post-processing will not see a moved-from value. Eg:
 
 ```c++
-MyStatus DoSomething() {
-  MyStatus status_no_nrvo;
+absl::Status DoSomething() {
+  absl::Status status_no_nrvo;
   // 'status' is a reference so NRVO and all associated optimizations
   // will be disabled.
   // The 'return status;' statements will always copy the object and Logger
   // will always see the correct value.
-  MyStatus& status = status_no_nrvo;
-  auto log_on_error = RunWhenOutOfScope([&status] {
+  absl::Status& status = status_no_nrvo;
+  auto log_on_error = absl::MakeCleanup([&status] {
     if (!status.ok()) LOG(ERROR) << status;
   });
   status = DoA();
@@ -123,19 +121,19 @@ MyStatus DoSomething() {
 }
 ```
 
-## Another example:
+## Another Real Life Example
 
 ```c++
 std::string EncodeVarInt(int i) {
   std::string out;
-  StringOutputStream string_output(&out);
-  CodedOutputStream coded_output(&string_output);
+  proto2::io::StringOutputStream string_output(&out);
+  proto2::io::CodedOutputStream coded_output(&string_output);
   coded_output.WriteVarint32(i);
   return out;
 }
 ```
 
-`CodedOutputStream` does some work in the destructor to trim unused trailing
+`CodedOutputStream` does some work on the destructor to trim unused trailing
 bytes. This function can leave garbage bytes on the string if NRVO does not
 happen.
 
@@ -149,8 +147,8 @@ the block is finished. Like this:
 std::string EncodeVarInt(int i) {
   std::string out;
   {
-    StringOutputStream string_output(&out);
-    CodedOutputStream coded_output(&string_output);
+    proto2::io::StringOutputStream string_output(&out);
+    proto2::io::CodedOutputStream coded_output(&string_output);
     coded_output.WriteVarint32(i);
   }
   // At this point the streams are destroyed and they already flushed.