mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joris Van Remoortere" <joris.van.remoort...@gmail.com>
Subject Re: Review Request 33572: Add C++11 unrestricted union to the C++ style guide.
Date Wed, 29 Apr 2015 21:47:59 GMT


> On April 27, 2015, 4:51 a.m., Michael Park wrote:
> > docs/mesos-c++-style-guide.md, line 287
> > <https://reviews.apache.org/r/33572/diff/1/?file=942036#file942036line287>
> >
> >     On first thought, this sounded fine because I had `Option` in mind as the primary
use case.
> >     
> >     > Therefore, only use unrestricted unions (i.e., unions with non-POD types)
when the union has only a single field.
> >     
> >     On second thought however, for `Try` and `Result`, don't we want the following?
> >     
> >     ```
> >       union
> >       {
> >         T t;
> >         std::string message;
> >       };
> >     ```
> >     
> >     Is the strategy to keep the `std::string` out of there? or perhaps we can treat
`Option`, `Try` and `Result` as specialized use cases for now?

We discussed this in a meeting and agreed that for `Try` and `Result` we can still use union.
We can keep the error `std::string message;` outside of the union. The allocation of the string
will only happen if we actually set an error string, not by default. This is sufficient to
get the performance improvement we're after.


- Joris


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


On April 26, 2015, 8:26 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33572/
> -----------------------------------------------------------
> 
> (Updated April 26, 2015, 8:26 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Joris Van Remoortere, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary. We'll be updating 'Option', 'Result', etc with this for performance improvements
soon too!
> 
> 
> Diffs
> -----
> 
>   docs/mesos-c++-style-guide.md fe98f90ad0b0f5dd38af97e85062e90cee8de99e 
> 
> Diff: https://reviews.apache.org/r/33572/diff/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message