> 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
> 
>

Reply via email to