You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2015/06/02 11:34:56 UTC
Re: Review Request 33271: Update style guide to disallow capturing
temporaries by reference.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33271/
-----------------------------------------------------------
(Updated June 2, 2015, 9:34 a.m.)
Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael Park, and Till Toenshoff.
Changes
-------
Add preferred alias pattern for temporary of `T`.
Bugs: MESOS-2629
https://issues.apache.org/jira/browse/MESOS-2629
Repository: mesos
Description
-------
Follow up from r32630.
Diffs (updated)
-----
docs/mesos-c++-style-guide.md 13312f6f4fe1788791479bd768f60df0a8e80e69
Diff: https://reviews.apache.org/r/33271/diff/
Testing
-------
Thanks,
Joris Van Remoortere
Re: Review Request 33271: Update style guide to disallow capturing
temporaries by reference.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33271/#review86189
-----------------------------------------------------------
Ship it!
docs/mesos-c++-style-guide.md
<https://reviews.apache.org/r/33271/#comment138129>
I totally didn't follow this. ;-) How about something as simple as:
vector<string> strings{"hello"};
string& s = strings[0];
strings.erase(strings.begin());
s += "world"; // THIS IS A DANGLING REFERENCE!
- Benjamin Hindman
On June 2, 2015, 9:34 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33271/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 9:34 a.m.)
>
>
> Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael Park, and Till Toenshoff.
>
>
> Bugs: MESOS-2629
> https://issues.apache.org/jira/browse/MESOS-2629
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Follow up from r32630.
>
>
> Diffs
> -----
>
> docs/mesos-c++-style-guide.md 13312f6f4fe1788791479bd768f60df0a8e80e69
>
> Diff: https://reviews.apache.org/r/33271/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 33271: Update style guide to disallow capturing
temporaries by reference.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33271/#review86186
-----------------------------------------------------------
docs/mesos-c++-style-guide.md
<https://reviews.apache.org/r/33271/#comment138125>
What about this case:
T t("Hello")'
const T& tprime = t.Member();
- Benjamin Hindman
On June 2, 2015, 9:34 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33271/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 9:34 a.m.)
>
>
> Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael Park, and Till Toenshoff.
>
>
> Bugs: MESOS-2629
> https://issues.apache.org/jira/browse/MESOS-2629
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Follow up from r32630.
>
>
> Diffs
> -----
>
> docs/mesos-c++-style-guide.md 13312f6f4fe1788791479bd768f60df0a8e80e69
>
> Diff: https://reviews.apache.org/r/33271/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 33271: Update style guide to disallow capturing
temporaries by reference.
Posted by Joris Van Remoortere <jo...@gmail.com>.
Here is the review. I used std::string instead. Also cleaned up an unused
struct.
https://reviews.apache.org/r/35120
On Fri, Jun 5, 2015 at 9:58 AM, Joris Van Remoortere <
joris.van.remoortere@gmail.com> wrote:
> Hey BenM,
>
> BenH added spaces before the commit, but thanks for also catching it!
> I'll follow up with a small patch to change the POD to something like
> SlaveID.
>
> Can you shepherd that one? :-)
>
> Joris
>
> On Fri, Jun 5, 2015 at 8:03 AM, Ben Mahler <be...@gmail.com>
> wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/33271/
>>
>> Thanks guys!
>>
>>
>> docs/mesos-c++-style-guide.md
>> <https://reviews.apache.org/r/33271/diff/9/?file=976428#file976428line180> (Diff
>> revision 9)
>>
>> 180
>>
>> foreachpair(const int& key, hashset<int>& values, index) {}
>>
>> 181
>>
>> foreachvalue(const hashset<int>& values, index) {}
>>
>> 182
>>
>> foreachkey(const int& key, index) {}
>>
>> Do we really want to encourage taking a const& of a POD type? In general we have not been doing this, so it seems pretty inconsistent to put it in this example.
>>
>> Also, looks like we need a space before the openening parenthesis.
>>
>>
>> - Ben Mahler
>>
>> On June 2nd, 2015, 9:34 a.m. UTC, Joris Van Remoortere wrote:
>> Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad,
>> Michael Park, and Till Toenshoff.
>> By Joris Van Remoortere.
>>
>> *Updated June 2, 2015, 9:34 a.m.*
>> *Bugs: * MESOS-2629 <https://issues.apache.org/jira/browse/MESOS-2629>
>> *Repository: * mesos
>> Description
>>
>> Follow up from r32630.
>>
>> Diffs
>>
>> - docs/mesos-c++-style-guide.md
>> (13312f6f4fe1788791479bd768f60df0a8e80e69)
>>
>> View Diff <https://reviews.apache.org/r/33271/diff/>
>>
>
>
Re: Review Request 33271: Update style guide to disallow capturing
temporaries by reference.
Posted by Joris Van Remoortere <jo...@gmail.com>.
Hey BenM,
BenH added spaces before the commit, but thanks for also catching it!
I'll follow up with a small patch to change the POD to something like
SlaveID.
Can you shepherd that one? :-)
Joris
On Fri, Jun 5, 2015 at 8:03 AM, Ben Mahler <be...@gmail.com>
wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33271/
>
> Thanks guys!
>
>
> docs/mesos-c++-style-guide.md
> <https://reviews.apache.org/r/33271/diff/9/?file=976428#file976428line180> (Diff
> revision 9)
>
> 180
>
> foreachpair(const int& key, hashset<int>& values, index) {}
>
> 181
>
> foreachvalue(const hashset<int>& values, index) {}
>
> 182
>
> foreachkey(const int& key, index) {}
>
> Do we really want to encourage taking a const& of a POD type? In general we have not been doing this, so it seems pretty inconsistent to put it in this example.
>
> Also, looks like we need a space before the openening parenthesis.
>
>
> - Ben Mahler
>
> On June 2nd, 2015, 9:34 a.m. UTC, Joris Van Remoortere wrote:
> Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad,
> Michael Park, and Till Toenshoff.
> By Joris Van Remoortere.
>
> *Updated June 2, 2015, 9:34 a.m.*
> *Bugs: * MESOS-2629 <https://issues.apache.org/jira/browse/MESOS-2629>
> *Repository: * mesos
> Description
>
> Follow up from r32630.
>
> Diffs
>
> - docs/mesos-c++-style-guide.md
> (13312f6f4fe1788791479bd768f60df0a8e80e69)
>
> View Diff <https://reviews.apache.org/r/33271/diff/>
>
Re: Review Request 33271: Update style guide to disallow capturing
temporaries by reference.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33271/#review86765
-----------------------------------------------------------
Thanks guys!
docs/mesos-c++-style-guide.md
<https://reviews.apache.org/r/33271/#comment138833>
Do we really want to encourage taking a const& of a POD type? In general we have not been doing this, so it seems pretty inconsistent to put it in this example.
Also, looks like we need a space before the openening parenthesis.
- Ben Mahler
On June 2, 2015, 9:34 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33271/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 9:34 a.m.)
>
>
> Review request for mesos, Bernd Mathiske, Cody Maloney, Joerg Schad, Michael Park, and Till Toenshoff.
>
>
> Bugs: MESOS-2629
> https://issues.apache.org/jira/browse/MESOS-2629
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Follow up from r32630.
>
>
> Diffs
> -----
>
> docs/mesos-c++-style-guide.md 13312f6f4fe1788791479bd768f60df0a8e80e69
>
> Diff: https://reviews.apache.org/r/33271/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joris Van Remoortere
>
>