[libcxx-commits] [PATCH] D154590: [libc++] mark barrier constructor as constexpr explicit in <barrier>

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this.



================
Comment at: libcxx/include/barrier:297
         return __barrier_base<_CompletionF>::max();
     }
----------------
Can you upload the new diff with context?


================
Comment at: libcxx/include/barrier:301
+    inline _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
+    constexpr explicit barrier(ptrdiff_t __count, _CompletionF __completion = _CompletionF())
         : __b_(__count, _VSTD::move(__completion)) {
----------------
constexpr is implicitly `inline`, the same for functions inlined in the class.


================
Comment at: libcxx/include/barrier:301
+    inline _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
+    constexpr explicit barrier(ptrdiff_t __count, _CompletionF __completion = _CompletionF())
         : __b_(__count, _VSTD::move(__completion)) {
----------------
Mordante wrote:
> constexpr is implicitly `inline`, the same for functions inlined in the class.
Can you add a test this is really `constexpr`?

Please use our typical `constexpr` test method (for example test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp`.
```
int main(int, char**) {
  test();

#if TEST_STD_VER > 17
  static_assert(test());
#endif

  return 0;
}
```


================
Comment at: libcxx/include/barrier:301
+    inline _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
+    constexpr explicit barrier(ptrdiff_t __count, _CompletionF __completion = _CompletionF())
         : __b_(__count, _VSTD::move(__completion)) {
----------------
Mordante wrote:
> Mordante wrote:
> > constexpr is implicitly `inline`, the same for functions inlined in the class.
> Can you add a test this is really `constexpr`?
> 
> Please use our typical `constexpr` test method (for example test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp`.
> ```
> int main(int, char**) {
>   test();
> 
> #if TEST_STD_VER > 17
>   static_assert(test());
> #endif
> 
>   return 0;
> }
> ```
For other reviewers I verified this wording was in the original paper https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1135r6.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154590