Possible mistake in abseil/tips #120

[yangvw]

I know this is unrelated to the main abseil corpus but I can't find another place to discuss the question. I'm also very new to C++ so please bear with me if I make any mistakes.

In https://abseil.io/tips/120, it discusses the influence of return values on RAII cleanup functions. It states that:
A refactor changes the last line from return status; to return MyStatus(); and suddenly the code stopped logging errors.
The effect of the change is that:
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.
However, the code of RunWhenOutOfScope() only makes reference the the status variable, not the temporary object returned by the function. Then it should have no chance of seeing the moved-from temporary object.

[tituswinters]

In that example, `RunWhenOutOfScope` is an RAII object that takes a callable (the lambda). That lambda is capturing a *reference* to the outer MyStatus object. That is, there is only one MyStatus object. The RAII object is cleaned up after the evaluation of the `return` statement, and as the return is by-move, `status` is moved-from by the time that RAII object is destroyed and the lambda is invoked. Does that make sense?

…

On Tue, Sep 26, 2023 at 11:37 PM yangvw ***@***.***> wrote: I know this is unrelated to the main abseil corpus but I can't find another place to discuss the question. I'm also very new to C++ so please bear with me if I make any mistakes. In https://abseil.io/tips/120, it discusses the influence of return values on RAII cleanup functions. It states that: A refactor changes the last line from return status; to return MyStatus(); and suddenly the code stopped logging errors. The effect of the change is that: 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. However, the code of RunWhenOutOfScope() only makes reference the the status variable, not the temporary object returned by the function. Then it should have no chance of seeing the moved-from temporary object. — Reply to this email directly, view it on GitHub <#1539>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC7CRX2TEJEL4VL27LJFYE3X4ONRPANCNFSM6AAAAAA5IVQRJE> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

-- *If you get an email from me outside of the 9-5 it is *not* because I'm always on or expect an immediate response from you; it is because of work flexibility <http://www.inc.com/john-boitnott/how-flexible-hours-can-create-a-better-work-life-balance.html> . Evening and weekend emails are a sign I allocated some regular working hours for other things (such as family, gym, friends,...). And I encourage you to feel free to do the same.

[yangvw]

The example seems to refer to the case with return MyStatus() when it states that a move happens across the function boundary. As for the case with return status, copy elision applies and status then is not a moved-from object.

[tituswinters]

Which example are you referring to, exactly?

…

On Thu, Sep 28, 2023 at 1:58 AM yangvw ***@***.***> wrote: The example seems to refer to the case with return MyStatus() when it states that a move happens across the function boundary. As for the case with return status, copy elision applies and status then is not a moved-from object. — Reply to this email directly, view it on GitHub <#1539 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC7CRX6MDEVFAYMZFKBZKZLX4UGWXANCNFSM6AAAAAA5IVQRJE> . You are receiving this because you commented.Message ID: ***@***.***>

-- *If you get an email from me outside of the 9-5 it is *not* because I'm always on or expect an immediate response from you; it is because of work flexibility <http://www.inc.com/john-boitnott/how-flexible-hours-can-create-a-better-work-life-balance.html> . Evening and weekend emails are a sign I allocated some regular working hours for other things (such as family, gym, friends,...). And I encourage you to feel free to do the same.

[yangvw]

I'm referring to the statement "A refactor changes the last line from return status; to return MyStatus(); and suddenly the code stopped logging errors." in the beginning of article. In the statement return MyStatus();, the variable status is not moved from as claimed by the article.

[tituswinters]

Ah, it's subtle. I think it's technically correct, but it could certainly be phrased more cleanly.

Specifically it says "returned variable is being moved into the return value." There are two possible returned values: status in the case that there was an error in DoA(), DoB(), or DoC(), OR the newly default-constructed MyStatus if it makes it to the last line (the success case).

The logging is only intended to trigger in one of the error cases. However, for all of the error lines, status is moved-from by the time the RAII cleanup object is invoked. In the success case, d'tors are running in reverse order after the return value: status is fine (but OK).

Does that clarify? I'll readily admit it's confusing - if there's a better way to explain the issue, I'd gladly review a PR.

[yangvw]

Ah, I see. I think to clarify it suffices to fix "The Problem" section, originally:

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.

The confusing part is that in fact it's not always true that the returned value is moved in the after code, but only if an error case is triggered. So the statement can be broken down to:

In the after code, copy elision is not being applied. In case that status is returned (i.e. an error is triggered), the returned variable is moved into the return value, and RunWhenOutOfScope() runs after the move operation is done and it is seeing a moved-from MyStatus object.

The original statement is technically correct in that it is referring to "the returned variable", which excludes the case where a temporary is returned, but this can be easily overlooked by C++ noobs such as myself and lead to the wrong idea.