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