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

[Hal Finkel via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D25403#580439, @jlebar wrote:

> > I'm not sure about that. It seems like a useful feature for the builtins to have. Logically speaking, they should be constexpr.
>
> I agree that it's logically correct for the builtins to be constexpr-evaluatable.  My point is just that doing this work and then writing a test doesn't buy us much in terms of ensuring that CUDA compilation doesn't break due to changes to libc++.


True, it is not an e2e test, but it could insure that this is not the problem.

> 
> 
>>> In addition, if I understand you correctly, we wouldn't be able to test all of the functions here, only the ones that call builtins.
>> 
>> What do you mean?
> 
> This patch adds constexpr to six function definitions, but only three of them directly call a builtin.  If I understand your proposal correctly, the other three function definitions would remain not-constexpr-evaluatable, and thus untested.

Well, we don't know about the others because it would depend on what function was found by ADL (potentially). Given that we only use these functions right now in <complex>, and how <complex> behaves for non-builtin floating-point types is not defined, I suspect this is much less concerning.


https://reviews.llvm.org/D25403