Add stream logging support for base::Optional

Add stream logging support for base::Optional

Other base types, such as base::TimeDelta, provide operator<< for use
when logging, for example:

LOG(ERROR) << "Some log message: " << time_delta;

As we migrate some uses of base::TimeDelta to
base::Optional<base::TimeDelta>, I'm finding that logged base::TimeDelta
instances won't compile without this change.

BUG=625132

[Bryan McQuade, 2016-07-01 12:24:17]

bmcquade@chromium.org changed reviewers:
+ dcheng@chromium.org

[Bryan McQuade, 2016-07-01 12:24:24]

PTAL

[mlamouri (slow - plz ping), 2016-07-01 12:35:01]

mlamouri@chromium.org changed reviewers:
+ danakj@chromium.org, mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-07-01 12:35:02]

+danakj@ who reviewed the initial base/optional.h change.

[dcheng, 2016-07-01 12:58:29]

std::optional doesn't appear to be streamable, which will present a problem
if/when we migrate to the standard version.

I'm guessing the things that fail to build are LOG(...) calls rather than
(D)CHECKs?

[Bryan McQuade, 2016-07-01 13:00:42]

On 2016/07/01 at 12:58:29, dcheng wrote:
> std::optional doesn't appear to be streamable, which will present a problem
if/when we migrate to the standard version.
> 
> I'm guessing the things that fail to build are LOG(...) calls rather than
(D)CHECKs?

Ah, if we're trying to be consistent with std::optional, then this may not make
sense. I can probably work around it - just requires writing code like:

LOG(ERROR) << "Message: " << opt ? opt.value() : "(unset)";

at the call sites, which is a little ugly.

Is there another way we could accomplish this w/o having to inline the stream
implementation at each call site?

[mlamouri (slow - plz ping), 2016-07-01 13:01:09]

On 2016/07/01 at 12:58:29, dcheng wrote:
> std::optional doesn't appear to be streamable, which will present a problem
if/when we migrate to the standard version.
> 
> I'm guessing the things that fail to build are LOG(...) calls rather than
(D)CHECKs?

std::optional not having operator<< would be a concern. Though, I assumed we
could simply implement the same operator<< with std::optional<T> if we care
about this feature. Am I making wrong assumptions here?

[mlamouri (slow - plz ping), 2016-07-01 13:03:05]

... though, maybe that shouldn't be in base/optional.h but base/logging.h? We
seem to have at least one operator<< defined there for a std type.

[Bryan McQuade, 2016-07-01 13:05:20]

+1 to adding this to base/logging.h, and then updating to std::optional
when we migrate to it. Daniel, Dana, how does that sound to you?

On Fri, Jul 1, 2016 at 9:03 AM <mlamouri@chromium.org> wrote:

> ... though, maybe that shouldn't be in base/optional.h but base/logging.h?
> We
> seem to have at least one operator<< defined there for a std type.
>
> https://codereview.chromium.org/2109063006/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Bryan McQuade, 2016-07-01 19:06:39]

On 2016/07/01 at 13:05:20, Bryan McQuade wrote:
> +1 to adding this to base/logging.h, and then updating to std::optional
> when we migrate to it. Daniel, Dana, how does that sound to you?
> 
> On Fri, Jul 1, 2016 at 9:03 AM <mlamouri@chromium.org> wrote:
> 
> > ... though, maybe that shouldn't be in base/optional.h but base/logging.h?
> > We
> > seem to have at least one operator<< defined there for a std type.
> >
> > https://codereview.chromium.org/2109063006/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to chromium-reviews+unsubscribe@chromium.org.
> 

Turns out we do make extensive use of operator<< for Optional<TimeDelta>, but
only in a single .cc file. So for now I think I can drop the implementation for
this into an anonymous namespace in that cc file. Happy to pursue this change
further if we think it's generally useful, but I can also abandon it if
preferred.

[danakj, 2016-07-01 20:16:06]

On Fri, Jul 1, 2016 at 12:06 PM, <bmcquade@chromium.org> wrote:

> On 2016/07/01 at 13:05:20, Bryan McQuade wrote:
> > +1 to adding this to base/logging.h, and then updating to std::optional
> > when we migrate to it. Daniel, Dana, how does that sound to you?
> >
> > On Fri, Jul 1, 2016 at 9:03 AM <mlamouri@chromium.org&gt; wrote:
> >
> > > ... though, maybe that shouldn't be in base/optional.h but
> base/logging.h?
> > > We
> > > seem to have at least one operator<< defined there for a std type.
> > >
> > > https://codereview.chromium.org/2109063006/
> > >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
> >
>
> Turns out we do make extensive use of operator<< for Optional<TimeDelta>,
> but
> only in a single .cc file. So for now I think I can drop the
> implementation for
> this into an anonymous namespace in that cc file. Happy to pursue this
> change
> further if we think it's generally useful, but I can also abandon it if
> preferred.
>

That sounds fine for now.

We do want the API to match the std one so we can swap it out when c++14
arrives in our toolchains. While we could put it in logging.h, I really
don't want to add more lines of code to that file if I can help it. And
logging the .value() is probably good enough normally too?


>
> https://codereview.chromium.org/2109063006/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Bryan McQuade, 2016-07-01 20:58:37]

On 2016/07/01 at 20:16:06, danakj wrote:
> On Fri, Jul 1, 2016 at 12:06 PM, <bmcquade@chromium.org> wrote:
> 
> > On 2016/07/01 at 13:05:20, Bryan McQuade wrote:
> > > +1 to adding this to base/logging.h, and then updating to std::optional
> > > when we migrate to it. Daniel, Dana, how does that sound to you?
> > >
> > > On Fri, Jul 1, 2016 at 9:03 AM <mlamouri@chromium.org&gt; wrote:
> > >
> > > > ... though, maybe that shouldn't be in base/optional.h but
> > base/logging.h?
> > > > We
> > > > seem to have at least one operator<< defined there for a std type.
> > > >
> > > > https://codereview.chromium.org/2109063006/
> > > >
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an
> > email to chromium-reviews+unsubscribe@chromium.org.
> > >
> >
> > Turns out we do make extensive use of operator<< for Optional<TimeDelta>,
> > but
> > only in a single .cc file. So for now I think I can drop the
> > implementation for
> > this into an anonymous namespace in that cc file. Happy to pursue this
> > change
> > further if we think it's generally useful, but I can also abandon it if
> > preferred.
> >
> 
> That sounds fine for now.
> 
> We do want the API to match the std one so we can swap it out when c++14
> arrives in our toolchains. While we could put it in logging.h, I really
> don't want to add more lines of code to that file if I can help it. And
> logging the .value() is probably good enough normally too?
> 
> 
> >
> > https://codereview.chromium.org/2109063006/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to chromium-reviews+unsubscribe@chromium.org.
> 

Sounds good, will close this out.

[dcheng, 2016-07-02 00:26:51]

On 2016/07/01 20:58:37, Bryan McQuade wrote:
> On 2016/07/01 at 20:16:06, danakj wrote:
> > On Fri, Jul 1, 2016 at 12:06 PM, <mailto:bmcquade@chromium.org> wrote:
> > 
> > > On 2016/07/01 at 13:05:20, Bryan McQuade wrote:
> > > > +1 to adding this to base/logging.h, and then updating to std::optional
> > > > when we migrate to it. Daniel, Dana, how does that sound to you?
> > > >
> > > > On Fri, Jul 1, 2016 at 9:03 AM <mailto:mlamouri@chromium.org&gt; wrote:
> > > >
> > > > > ... though, maybe that shouldn't be in base/optional.h but
> > > base/logging.h?
> > > > > We
> > > > > seem to have at least one operator<< defined there for a std type.
> > > > >
> > > > > https://codereview.chromium.org/2109063006/
> > > > >
> > > >
> > > > --
> > > > You received this message because you are subscribed to the Google
Groups
> > > "Chromium-reviews" group.
> > > > To unsubscribe from this group and stop receiving emails from it, send
an
> > > email to mailto:chromium-reviews+unsubscribe@chromium.org.
> > > >
> > >
> > > Turns out we do make extensive use of operator<< for Optional<TimeDelta>,
> > > but
> > > only in a single .cc file. So for now I think I can drop the
> > > implementation for
> > > this into an anonymous namespace in that cc file. Happy to pursue this
> > > change
> > > further if we think it's generally useful, but I can also abandon it if
> > > preferred.
> > >
> > 
> > That sounds fine for now.
> > 
> > We do want the API to match the std one so we can swap it out when c++14
> > arrives in our toolchains. While we could put it in logging.h, I really
> > don't want to add more lines of code to that file if I can help it. And
> > logging the .value() is probably good enough normally too?
> > 
> > 
> > >
> > > https://codereview.chromium.org/2109063006/
> > >
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email to mailto:chromium-reviews+unsubscribe@chromium.org.
> > 
> 
> Sounds good, will close this out.

Just to followup, we can't (easily) add operator<< for std::optional without
triggering undefined behavior.

From http://en.cppreference.com/w/cpp/language/extending_std:
"It is allowed to add template specializations for any standard library template
to the namespace std only if the declaration depends on a user-defined type and
the specialization satisfies all requirements for the original template, except
where such specializations are prohibited."