diff --git a/_posts/2018-04-20-totw-120.md b/_posts/2018-04-20-totw-120.md index 857e0c1..b9030fc 100644 --- a/_posts/2018-04-20-totw-120.md +++ b/_posts/2018-04-20-totw-120.md @@ -10,17 +10,17 @@ 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 a code snippet like below, which relies on a scope +guard utility, which seems to be working as expected: ```c++ -MyStatus DoSomething() { - MyStatus status; - auto log_on_error = RunWhenOutOfScope([&status] { +absl::Status DoSomething() { + absl::Status status; + absl::Cleanup log_on_error = [&status] { if (!status.ok()) LOG(ERROR) << status; - }); + }; status = DoA(); if (!status.ok()) return status; status = DoB(); @@ -32,7 +32,8 @@ MyStatus DoSomething() { ``` A refactor changes the last line from `return status;` to -`return MyStatus();` and suddenly the code stopped logging errors. +`return absl::OkStatus();` and suddenly the code stopped logging +errors. What is going on? @@ -56,16 +57,16 @@ 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 implicit moves. -The *before* code worked because copy elision is taking place and the +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. +unique instance of the `absl::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, copy elision is not being applied and the returned variable +is being moved into the return value. `absl::Cleanup` runs after the move +operation is done and it is seeing a moved-from `absl::Status` object. -Note that the *before* code was not correct either, as it relies on the copy +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 @@ -81,8 +82,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,8 +92,8 @@ 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; } @@ -103,16 +104,16 @@ 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.: ```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; + absl::Cleanup log_on_error = [&status] { if (!status.ok()) LOG(ERROR) << status; - }); + }; status = DoA(); if (!status.ok()) return status; status = DoB();