You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Bernd Mathiske <be...@mesosphere.io> on 2015/02/09 11:30:28 UTC

Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/
-----------------------------------------------------------

Review request for mesos.


Repository: mesos


Description
-------

Explicitly allows auto declarations for shared pointer creations. Example:

    auto nameList = make_shared<list<string>>();

Here the type on the left is already explicitly stated on the right. This replaces

    shared_ptr<list<string>> nameList = make_shared<list<string>>();

where we needlessly write the same type twice.


Diffs
-----

  docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 

Diff: https://reviews.apache.org/r/30792/diff/


Testing
-------

None. This is doc text only.


Thanks,

Bernd Mathiske


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Bernd Mathiske <be...@mesosphere.io>.
All,

as stated in the review request below, I propose that we update our C++ style guide to explicitly allow “auto” for shared pointer creations.

I suggest that any similar situation should be included. In general, if the exact same type that would be written on the left is already fully expressed on the right, then we should be allowed to use auto. Would you like me to give it a try to define this in the docs as well? Or would you rather prefer just the one-off example I posted in the review?

Bernd

> On Feb 9, 2015, at 11:30 AM, Bernd Mathiske <be...@mesosphere.io> wrote:
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30792/
> -----------------------------------------------------------
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Explicitly allows auto declarations for shared pointer creations. Example:
> 
>    auto nameList = make_shared<list<string>>();
> 
> Here the type on the left is already explicitly stated on the right. This replaces
> 
>    shared_ptr<list<string>> nameList = make_shared<list<string>>();
> 
> where we needlessly write the same type twice.
> 
> 
> Diffs
> -----
> 
>  docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 
> 
> Diff: https://reviews.apache.org/r/30792/diff/
> 
> 
> Testing
> -------
> 
> None. This is doc text only.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/#review71681
-----------------------------------------------------------


The shared_ptr -> shared_ptr auto case seems good.

The make_shared -> shared_ptr auto case seems a tiny bit non-obvious, but I guess it's reasonable for people to understand make_shared. It's going to be fairly uncommon in our code however, since we don't use shared_ptr heavily.

I will let benh give the ship it on this one.


docs/mesos-c++-style-guide.md
<https://reviews.apache.org/r/30792/#comment117546>

    How about s/nameList/names/ to discourage including type information redundantly in variable names?


- Ben Mahler


On Feb. 9, 2015, 10:30 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30792/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2015, 10:30 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Explicitly allows auto declarations for shared pointer creations. Example:
> 
>     auto nameList = make_shared<list<string>>();
> 
> Here the type on the left is already explicitly stated on the right. This replaces
> 
>     shared_ptr<list<string>> nameList = make_shared<list<string>>();
> 
> where we needlessly write the same type twice.
> 
> 
> Diffs
> -----
> 
>   docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 
> 
> Diff: https://reviews.apache.org/r/30792/diff/
> 
> 
> Testing
> -------
> 
> None. This is doc text only.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/#review71923
-----------------------------------------------------------

Ship it!


I think this is a great use of auto!

- Benjamin Hindman


On Feb. 10, 2015, 11:37 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30792/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2015, 11:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Explicitly allows auto declarations for shared pointer creations. Example:
> 
>     auto nameList = make_shared<list<string>>();
> 
> Here the type on the left is already explicitly stated on the right. This replaces
> 
>     shared_ptr<list<string>> nameList = make_shared<list<string>>();
> 
> where we needlessly write the same type twice.
> 
> 
> Diffs
> -----
> 
>   docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 
> 
> Diff: https://reviews.apache.org/r/30792/diff/
> 
> 
> Testing
> -------
> 
> None. This is doc text only.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Bernd Mathiske <be...@mesosphere.io>.

> On Feb. 26, 2015, 5:38 p.m., Ben Mahler wrote:
> > Is make_shared available in tr1? (stout's memory.hpp still assumes tr1 or *full* c++11, so if we support compilers that have partial C++11 features this might be a bit confusing: we'd have includes for both <memory> and <stout/memory.hpp>).

No, make_shared is not in tr1. And we are not using <boost/make_shared.hpp>. And if we included this, it might get even more confusing with interim step compilers. For now, I will remove make_shared from my proposal.


- Bernd


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/#review74419
-----------------------------------------------------------


On Feb. 10, 2015, 3:37 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30792/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2015, 3:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Explicitly allows auto declarations for shared pointer creations. Example:
> 
>     auto nameList = make_shared<list<string>>();
> 
> Here the type on the left is already explicitly stated on the right. This replaces
> 
>     shared_ptr<list<string>> nameList = make_shared<list<string>>();
> 
> where we needlessly write the same type twice.
> 
> 
> Diffs
> -----
> 
>   docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 
> 
> Diff: https://reviews.apache.org/r/30792/diff/
> 
> 
> Testing
> -------
> 
> None. This is doc text only.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/#review74419
-----------------------------------------------------------


Is make_shared available in tr1? (stout's memory.hpp still assumes tr1 or *full* c++11, so if we support compilers that have partial C++11 features this might be a bit confusing: we'd have includes for both <memory> and <stout/memory.hpp>).

- Ben Mahler


On Feb. 10, 2015, 11:37 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30792/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2015, 11:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Explicitly allows auto declarations for shared pointer creations. Example:
> 
>     auto nameList = make_shared<list<string>>();
> 
> Here the type on the left is already explicitly stated on the right. This replaces
> 
>     shared_ptr<list<string>> nameList = make_shared<list<string>>();
> 
> where we needlessly write the same type twice.
> 
> 
> Diffs
> -----
> 
>   docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 
> 
> Diff: https://reviews.apache.org/r/30792/diff/
> 
> 
> Testing
> -------
> 
> None. This is doc text only.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/#review74676
-----------------------------------------------------------


Patch looks great!

Reviews applied: [30792]

All tests passed.

- Mesos ReviewBot


On Feb. 28, 2015, 2:44 p.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30792/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2015, 2:44 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Explicitly allows auto declarations for shared pointer creations. Example:
> 
>     auto nameList = shared_ptr<list<string>>(new list<string>());
> 
> Here the type on the left is already explicitly stated on the right. This replaces
> 
>     shared_ptr<list<string>> nameList = shared_ptr<list<string>>(new list<string>());
> 
> where we needlessly write the same type twice.
> 
> 
> Diffs
> -----
> 
>   docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 
> 
> Diff: https://reviews.apache.org/r/30792/diff/
> 
> 
> Testing
> -------
> 
> None. This is doc text only.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/
-----------------------------------------------------------

(Updated Feb. 28, 2015, 6:44 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen.


Changes
-------

Not using make_shared in this proposal.


Repository: mesos


Description (updated)
-------

Explicitly allows auto declarations for shared pointer creations. Example:

    auto nameList = shared_ptr<list<string>>(new list<string>());

Here the type on the left is already explicitly stated on the right. This replaces

    shared_ptr<list<string>> nameList = shared_ptr<list<string>>(new list<string>());

where we needlessly write the same type twice.


Diffs (updated)
-----

  docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 

Diff: https://reviews.apache.org/r/30792/diff/


Testing
-------

None. This is doc text only.


Thanks,

Bernd Mathiske


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/#review71798
-----------------------------------------------------------


Patch looks great!

Reviews applied: [30792]

All tests passed.

- Mesos ReviewBot


On Feb. 10, 2015, 11:37 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30792/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2015, 11:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Explicitly allows auto declarations for shared pointer creations. Example:
> 
>     auto nameList = make_shared<list<string>>();
> 
> Here the type on the left is already explicitly stated on the right. This replaces
> 
>     shared_ptr<list<string>> nameList = make_shared<list<string>>();
> 
> where we needlessly write the same type twice.
> 
> 
> Diffs
> -----
> 
>   docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 
> 
> Diff: https://reviews.apache.org/r/30792/diff/
> 
> 
> Testing
> -------
> 
> None. This is doc text only.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/
-----------------------------------------------------------

(Updated Feb. 10, 2015, 3:37 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen.


Changes
-------

Addressed issue. Expanded a bit on the general case.


Repository: mesos


Description
-------

Explicitly allows auto declarations for shared pointer creations. Example:

    auto nameList = make_shared<list<string>>();

Here the type on the left is already explicitly stated on the right. This replaces

    shared_ptr<list<string>> nameList = make_shared<list<string>>();

where we needlessly write the same type twice.


Diffs (updated)
-----

  docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 

Diff: https://reviews.apache.org/r/30792/diff/


Testing
-------

None. This is doc text only.


Thanks,

Bernd Mathiske


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/#review71633
-----------------------------------------------------------

Ship it!


IMO, a very good example!

- Alexander Rukletsov


On Feb. 9, 2015, 10:30 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30792/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2015, 10:30 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Explicitly allows auto declarations for shared pointer creations. Example:
> 
>     auto nameList = make_shared<list<string>>();
> 
> Here the type on the left is already explicitly stated on the right. This replaces
> 
>     shared_ptr<list<string>> nameList = make_shared<list<string>>();
> 
> where we needlessly write the same type twice.
> 
> 
> Diffs
> -----
> 
>   docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 
> 
> Diff: https://reviews.apache.org/r/30792/diff/
> 
> 
> Testing
> -------
> 
> None. This is doc text only.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>


Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30792/#review71620
-----------------------------------------------------------


Patch looks great!

Reviews applied: [30792]

All tests passed.

- Mesos ReviewBot


On Feb. 9, 2015, 10:30 a.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30792/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2015, 10:30 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Explicitly allows auto declarations for shared pointer creations. Example:
> 
>     auto nameList = make_shared<list<string>>();
> 
> Here the type on the left is already explicitly stated on the right. This replaces
> 
>     shared_ptr<list<string>> nameList = make_shared<list<string>>();
> 
> where we needlessly write the same type twice.
> 
> 
> Diffs
> -----
> 
>   docs/mesos-c++-style-guide.md b24fe4b64a2577060f66ef49ae8b5d4a9d94167c 
> 
> Diff: https://reviews.apache.org/r/30792/diff/
> 
> 
> Testing
> -------
> 
> None. This is doc text only.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>