Adopt absl::optional

[jsedgwick]

Envoy's custom Optional is anemic compared to absl (https://github.com/abseil/abseil-cpp/blob/master/absl/types/optional.h) and folly (https://github.com/facebook/folly/blob/master/folly/Optional.h)

For instance, envoy Optional doesn't have move ctors and doesn't do some finger-saving type inference.

Replace with one of the above, presumably absl's since that's already an envoy dep.
AFAICT the big-picture difference between absl's and folly's is that absl's is a drop-in replacement for std::optional (C++17). Folly's has more bells and whistles but given that envoy has been getting away with the current impl, absl is a better call.

[dnoe]

Obviously I'm biased but I'd love to see us switch to absl::optional. This would be a nice project for a newcomer, too.

[jsedgwick]

I will take care of it when I can unless someone else snatches it up.

Don't be too surprised if I advocate for some folly components later on, though 😉 Especially Singleton, Synchronized, ThreadLocal, and the libevent wrappers.

[dio]

👍🏻

[mattklein123]

In this case I think we should definitely use absl::Optional. @jsedgwick my advice would be to not worry about this and per @dnoe leave it for a new contributor as it is a very easy item and a good first task. The current Optional that we have is clearly not as fleshed out as the others but it works fine for the cases in which we use it. As per Folly, I'm not super interested in depending on another utility library unless there is a very good reason but we can discuss those on a case by case basis.

[jsedgwick]

@mattklein123 I am a new contributor :) But I have plenty on my plate so can leave it for now.

As for everything else, nothing is urgent and we can discuss once you're back. The TLDR of my viewpoint, though, is that depending on a few actively maintained components from an external library in order not to have to maintain our own is worth the cost of setting up the dependency. Especially since I would bet my bottom dollar that some of these components (esp libevent wrappers) will be very low hanging fruit for long term perf work. And the ones I have in mind are safer and less verbose to use (viz Singleton, ThreadLocal), so would have immediate code cleanliness impact. Again, this is a longer discussion to be had case by case that we can punt on for now.

[srikailash]

I added absl::optional to envoy in this commit (https://github.com/srikailash/envoy/commit/9e8b79fdad756f859435c28bcc4c28de2980dc01) following https://github.com/envoyproxy/envoy/blob/master/bazel/EXTERNAL_DEPS.md. for further work on this issue do we need to implement a wrapper around absl::optional in envoy namespace?

[ggreenway]

No, I don't think we should have any type of wrapper. I think we just need to replace all uses of Optional with absl::optional, and delete the existing implementation.

[srikailash]

I added basic test to using absl:optional here https://github.com/srikailash/envoy/commit/e9dd4193766f674fc94b1fd7a29a601702a6342f. Can work on replacing if that's the way to go.

[htuch]

+1 on replacing. @srikailash it's easier to review if you just file a PR, thanks.

[srikailash]

@htuch, sure. Wanted to check approach first. Thanks for the post on managing external dependencies with bazel(https://blog.envoyproxy.io/external-c-dependency-management-in-bazel-dd37477422f5)