[libc-commits] [PATCH] D129920: [libc] Trivial implementation of std::optional

[Jeff Bailey via Phabricator via libc-commits]

jeffbailey added inline comments.


================
Comment at: libc/src/__support/CPP/Optional.h:44
+
+  Optional(const Optional &t) : StoredValue(t.value()), InUse(true) {}
+
----------------
zero9178 wrote:
> This copy constructor causes undefined behaviour in the case that `t` is not does not have a value. Shouldn't this be conditionally copying `t.value()` into `StoredValue` depending on whether `t.has_value()`?
> 
> (I am an outsider to the libc part of LLVM, but I am guessing you'll probably also want a copy assignment operator acting similarily)
Oh bugger.  This probably explains why everyone does the storage as a sub object.

Happily, I also found an implementation in llvm/include/llvm/ADT/optional.h that's a bit simpler than the one in libcxx, so I'll go with that.  Gimme another day.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129920/new/

https://reviews.llvm.org/D129920