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

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet added a comment.

You'd also need to adjust the Bazel config <https://github.com/llvm/llvm-project/blob/main/utils/bazel/llvm-project-overlay/libc/BUILD.bazel>.



================
Comment at: libc/src/__support/CPP/Optional.h:15
+
+// Trivial Nullopt_t struct.
+struct Nullopt_t {};
----------------
According to our current style guide that would be `NulloptT`.
Note that I would be in favor of using the STL names whenever we're re-implementing them but that would be a separate discussion.

Do you want to limit it to POD types? Or to primitive types?


================
Comment at: libc/src/__support/CPP/Optional.h:21-24
+// This is very simple implementation of the std::optional class.  There are a
+// number of guarantees in the standard that are not made here.
+//
+// This class will be extended as needed in future.
----------------
I would list the requirements for `T` here, AFAICT:
 - default constructible
 - copy constructible
 - copy assignable
 - destructible


================
Comment at: libc/src/__support/CPP/Optional.h:47
+
+  T value() const noexcept { return StoredValue; }
+
----------------
You can drop `noexpect` here and below, exceptions are [[ https://github.com/llvm/llvm-project/blob/9234a7c0dfa38bd3855a11a57a9a645fcef7ef79/libc/cmake/modules/LLVMLibCObjectRules.cmake#L16-L30 | disabled throughout llvm-libc ]]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129920