[libcxx-commits] [PATCH] D65034: [libc++] use constexpr for sizeof comparisons
Alan Somers via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Mar 25 09:43:35 PDT 2020
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
- if (sizeof(_Fun) <= sizeof(__buf_) &&
+ if _LIBCPP_CONSTEXPR_AFTER_CXX14(sizeof(_Fun) <= sizeof(__buf_) &&
> 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.
CHANGES SINCE LAST ACTION
More information about the libcxx-commits