[
https://issues.apache.org/jira/browse/MESOS-2629?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Adam B updated MESOS-2629:
--------------------------
Sprint: Mesosphere Q2 Sprint 8 - 5/1, Mesosphere Q1 Sprint 9 - 5/15,
Mesosphere Q1 Sprint 10 - 5/30 (was: Mesosphere Q2 Sprint 8 - 5/1, Mesosphere
Q1 Sprint 9 - 5/15)
> Update style guide to disallow capture by reference of temporaries
> ------------------------------------------------------------------
>
> Key: MESOS-2629
> URL: https://issues.apache.org/jira/browse/MESOS-2629
> Project: Mesos
> Issue Type: Task
> Components: documentation, technical debt
> Reporter: Joris Van Remoortere
> Assignee: Joris Van Remoortere
>
> We modify the style guide to disallow constant references to temporaries as a
> whole. This means disallowing both (1) and (2) below.
> h3. Background
> 1. Constant references to simple expression temporaries do extend the
> lifetime of the temporary till end of function scope:
> * Temporary returned by function:
> {code}
> // See full example below.
> T f(const char* s) { return T(s); }
> {
> const T& good = f("Ok");
> // use of good is ok.
> }
> {code}
> * Temporary constructed as simple expression:
> {code}
> // See full example below.
> {
> const T& good = T("Ok");
> // use of good is ok.
> }
> {code}
> 2. Constant references to expressions that result in a reference to a
> temporary do not extend the lifetime of the temporary:
> * Temporary returned by function:
> {code}
> // See full example below.
> T f(const char* s) { return T(s); }
> {
> const T& bad = f("Bad!").Member();
> // use of bad is invalid.
> }
> {code}
> * Temporary constructed as simple expression:
> {code}
> // See full example below.
> {
> const T& bad = T("Bad!").Member();
> // use of bad is invalid.
> }
> {code}
> h3. Mesos Case
> - In Mesos we use Future<T> a lot. Many of our functions return Futures by
> value:
> {code}
> class Socket {
> Future<Socket> accept();
> Future<size_t> recv(char* data, size_t size);
> ...
> }
> {code}
> - Sometimes we capture these Futures:
> {code}
> {
> const Future<Socket>& accepted = socket.accept(); // Valid c++, propose
> we disallow.
> }
> {code}
> - Sometimes we chain these Futures:
> {code}
> {
> socket.accept().then(lambda::bind(_accepted)); // Temporary will be valid
> during 'then' expression evaluation.
> }
> {code}
> - Sometimes we do both:
> {code}
> {
> const Future<Socket>& accepted =
> socket.accept().then(lambda::bind(_accepted)); // Dangerous! 'accepted'
> lifetime will not be valid till end of scope. Disallow!
> }
> {code}
> h3. Reasoning
> - Although (1) is ok, and considered a
> [feature|http://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/],
> (2) is extremely dangerous and leads to hard to track bugs.
> - If we explicitly allow (1), but disallow (2), then my worry is that someone
> coming along to maintain the code later on may accidentally turn (1) into
> (2), without recognizing the severity of this mistake. For example:
> {code}
> // Original code:
> const T& val = T();
> std::cout << val << std::endl;
> // New code:
> const T& val = T().removeWhiteSpace();
> std::cout << val << std::endl; // val could be corrupted since the destructor
> has been invoked and T's memory freed.
> {code}
> - If we disallow both cases: it will be easier to catch these mistakes early
> on in code reviews (and avoid these painful bugs), at the same cost of
> introducing a new style guide rule.
> h3. Performance Implications
> - BenH suggests c++ developers are commonly taught to capture by constant
> reference to hint to the compiler that the copy can be elided.
> - Modern compilers use a Data Flow Graph to make optimizations such as
> - *In-place-construction*: leveraged by RVO and NRVO to construct the
> object in place on the stack. Similar to "*Placement new*":
> http://en.wikipedia.org/wiki/Placement_syntax
> - *RVO* (Return Value Optimization):
> http://en.wikipedia.org/wiki/Return_value_optimization
> - *NRVO* (Named Return Value Optimization):
> https://msdn.microsoft.com/en-us/library/ms364057%28v=vs.80%29.aspx
> - Since modern compilers perform these optimizations, we no longer need to
> 'hint' to the compiler that the copies can be elided.
> h3. Example program
> {code}
> #include <stdio.h>
> class T {
> public:
> T(const char* str) : Str(str) {
> printf("+ T(%s)\n", Str);
> }
> ~T() {
> printf("- T(%s)\n", Str);
> }
> const T& Member() const
> {
> return *this;
> }
> private:
> const char* Str;
> };
> T f(const char* s) { return T(s); }
> int main() {
> const T& good = T("Ok");
> const T& good_f = f("Ok function");
> const T& bad = T("Bad!").Member();
> const T& bad_f = T("Bad function!").Member();
> printf("End of function scope...\n");
> }
> {code}
> Output:
> {code}
> + T(Ok)
> + T(Ok function)
> + T(Bad!)
> - T(Bad!)
> + T(Bad function!)
> - T(Bad function!)
> End of function scope...
> - T(Ok function)
> - T(Ok)
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
