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



I am not convinced we should add this. The alternative of using e.g., an 
`Option<T*>` or `Option<T const*>` seems to not only produce correct behavior 
(even when wrapping a ptr to `const`), but also caution users enough that 
noting here protects against dangling references or performs any reference 
lifetime extension. While this seems redundant in the case of `Option` where 
one could just return a `nullptr` for `None` values, such a pattern would 
translate seemlessly to e.g., `Try` or `Result`, and the behavior of empty case 
could be solved by documentation wherever we would return such a type. It would 
also avoid unusual semantics around assignment or comparision, and would e.g., 
continue to support hashing (the type proposed here does not support `hash`).

I'd suggest to drop this patch and instead use wrappers around pointers if we 
really want to provide such behavior in lieu of e.g., `contains` checks and 
returning naked values.

- Benjamin Bannier


On Sept. 23, 2018, 3:09 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68813/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2018, 3:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, and Michael Park.
> 
> 
> Bugs: MESOS-9252
>     https://issues.apache.org/jira/browse/MESOS-9252
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This adds support for options of references. While this is still
> under debate for `std::optional`, there are some use cases in
> stout that can benefit from this:
> 
>   // None if the value is not found, otherwise a reference
>   // to the value.
>   Option<T&> t = hashmap.get("key");
> 
> Assignment and equality are deleted in order to avoid confusion
> around which of the 2 possible behaviors they provide (e.g. are
> the references being compared? or are the objects being referred
> to being compared?)
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/option.hpp 
> 8feed012a55fed6eab89c883958324f3345e46e9 
> 
> 
> Diff: https://reviews.apache.org/r/68813/diff/1/
> 
> 
> Testing
> -------
> 
> Test added in subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Reply via email to