[libcxx-commits] [PATCH] D154110: [libc++] Implement LWG3843 (std::expected<T, E>::value() & assumes E is copy constructible)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/test/std/utilities/expected/expected.expected/observers/value.pass.cpp:23-39
+template <typename E>
+concept has_value =
+    requires(E&& e) {
+      { std::forward<E>(e).value() };
+    };
+
+struct OnlyCopyConstructibleErrorType {
----------------
I don't think this tests what you intend to test. If you remove `as_const` in the `&` overload, does this test suddenly fail? I don't think it would, since you're still providing the declaration of the member function.

Instead, I think what you want is this:

```
bool ref_called = true;
bool const_ref_called = true;

struct Foo {
  Foo(Foo const&) { const_ref_called = true; }
  Foo(Foo&) { ref_called = true; }
};

int main() {
  std::expected<int, Foo> e;
  try {
    e.value();
  catch (std::bad_expected_access const&) {
    assert(const_ref_called);
  }
}
```

Obviously this is a poor way to test this, but I think you see the point I'm trying to make?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154110