[PATCH] D128131: [ADT] Add has_value, value, value_or to llvm::Optional

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/ADT/Optional.h:208
 
+  T &value() &noexcept {
+    assert(hasVal);
----------------
`std::optional::value` has undesired exception checking semantics: https://en.cppreference.com/w/cpp/utility/optional/value
llvm-project defaults to not use exceptions so this is wasteful.

And see `_LIBCPP_AVAILABILITY_BAD_OPTIONAL_ACCESS` in libcxx/ which has `__attribute__((availability(macos,strict,introduced=10.13)))`/etc. It seems that Apple contributors no longer test such an old SDK (we haven't heard of complaints about unavailablity of value, but Chrome toolchain folks do target 10.12).

`value` likely needs a `LLVM_DEPRECATED`. See https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716/19?u=maskray


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128131