[libcxx-commits] [PATCH] D65034: [libc++] use constexpr for sizeof comparisons

[Alan Somers via Phabricator via libcxx-commits]

asomers added a comment.

In D65034#1941477 <https://reviews.llvm.org/D65034#1941477>, @ldionne wrote:

> Looking at this again and the corresponding GCC bug report, I think we should either:
>
> - not fix this issue -- this warning shouldn't bubble up since we mark our headers as system headers, or
> - disable `-Wplacement-new` around that code with a `#pragma`, and reference the GCC bug in a comment.


Well, the warning does bubble up.  I ran into this when using GCC to build FreeBSD for riscv64.  The only way I could get it to build was by adding `-Wno-placement-new -Wno-attributes`.  See https://svnweb.freebsd.org/base/head/tests/sys/fs/fusefs/Makefile?r1=350162&r2=350163& .  And while we could just disable that warning, that doesn't feel right, because the GCC people don't consider this a bug at all.  They say "if constexpr is of course the right solution here, for certain".



================
Comment at: include/functional:1769
             _FunAlloc __af(__a);
-            if (sizeof(_Fun) <= sizeof(__buf_) &&
+            if _LIBCPP_CONSTEXPR_AFTER_CXX14(sizeof(_Fun) <= sizeof(__buf_) &&
                 is_nothrow_copy_constructible<_Fp>::value &&
----------------
ldionne wrote:
> asomers wrote:
> > ldionne wrote:
> > > The purpose of `_LIBCPP_CONSTEXPR_AFTER_CXX14` isn't to be used that way, it's meant for Standard Library facilities that were marked `constexpr` in C++17. I think we should have a macro specifically for constructing an `if constexpr` when supported.
> > How about something like this:
> > ```
> > #ifdef __cpp_if_constexpr
> > #define __IF_CONSTEXPR constexpr
> > #else
> > #define __IF_CONSTEXPR
> > #endif
> > ```
> Yes, except it should be `#define _LIBCPP_IF_CONSTEXPR`. I'm also unsure about this whole thing: for anything that requires `if constexpr` for correctness, we can't use `_LIBCPP_IF_CONSTEXPR` (because it's required for correctness). So this will only be useful for cases where we want to use `if constexpr` instead of just `if` as a QOI matter, and it's not clear to me this is worth introducing a macro. So I'm kind of ambivalent between `_LIBCPP_IF_CONSTEXPR` and just having the `#if __cpp_if_constexpr` check right there in the code. Thoughts?
Whether or not to introduce a macro is strictly a stylistic matter, and you're the one whose opinion on style matters.  I'll do it as you suggest.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D65034