[PATCH] D25403: [CUDA] Mark __libcpp_{isnan, isinf, isfinite} as constexpr.

[Justin Lebar via cfe-commits]

jlebar added a comment.

Thank you, Marshall.

In https://reviews.llvm.org/D25403#572998, @mclow.lists wrote:

> My build setup is similar to yours (on Mac OS X):
>
>   cd $LLVM_BUILD ; rm -rf libcxx ; mkdir libcxx ; cd libcxx 
>   CXX=$LLVM_BIN/clang++ cmake -DLLVM_PATH=$LLVM/llvm -DLIBCXX_CXX_ABI=libcxxabi -DLIBCXX_CXX_ABI_INCLUDE_PATHS=/usr/include $LIBCXX
>   make              # build libc++
>   make check-libcxx # run the tests


Ah, it's -DLIBCXX_CXX_ABI and -DLIBCXX_CXX_ABI_INCLUDE_PATHS that I was missing.

Is this the recommended way of building libcxx?  Should I update the documentation?

Likewise, should I update the documentation to indicate that check-cxx may fail with a clang built from tip of tree, due to c++17 support being experimental?  Or do you all want to change the target so that it doesn't run the c++17 tests by default?  This burned about an hour of developer time yesterday, I guess because we couldn't believe that check-cxx would be intentionally broken like that, so it would be nice to have some sort of fix.

> I don't see how this can possibly be constexpr.
>  it calls isfinite(), which is hoisted from ::isfinite(), which comes from the C library.
>  Since C knows nothing about constexpr, we're stuck.

Functions that call non-constexpr things can be marked constexpr; you just can't evaluate them in a constexpr context (as you demonstrated).

All I need is for the function to be marked as constexpr; I do not need the function be constexpr-evaluatable.

I think the implications of this change would be that, if you evaluated this function in a constexpr context, before the change would be a compile error when calling __libcpp_isnan, and after this change it would be a compile error at calling ::isnan.  Since the function should not be called outside of libc++ anyway, I was hoping that wouldn't make a difference to anyone.


https://reviews.llvm.org/D25403