[libcxx-commits] [PATCH] D102119: [libcxx][optional] adds missing constexpr operations

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM if CI passes, and the two nits applied.



================
Comment at: libcxx/test/support/archetypes.h:56
 
-    static void reset_constructors() {
+    TEST_CONSTEXPR_CXX20 static void reset_constructors() {
       constructed = value_constructed = default_constructed =
----------------
Quuxplusone wrote:
> Style nit 1: `static TEST_CONSTEXPR_CXX20 void`
> Correctness nit 2: None of these functions that touch static members like `alive` and `constructed` can actually be constexpr in C++20. Constexpr functions aren't allowed to modify (or even read) global data. Therefore, adding `TEST_CONSTEXPR_CXX20` to all these functions is really just noise; you could remove it and simplify this patch.
Agree with both.

I didn't care about the placement of `static` vs `TEST_CONSTEXPR_CXX20`, but I grepped and it is more consistent with existing style to use `static TEST_CONSTEXPR_CXX20` than the reverse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102119