Hi there,
just a (stupid?) question: why does TMath::IsNaN() return Int_t instead of Bool_t?
Reason for the question: I ran clang-tidy over my code and it showed
myfile.cpp:93:13: warning: implicit cast 'Int_t' -> bool [readability-implicit-bool-cast]
if (TMath::IsNaN(score)) {
^
!= 0
In general, I like this checker very much, but obviously I don’t want to add a !=0 check to IsNaN!
At first it looks like a legacy code and sloppy programming to me. IsNaN, depending on configuration (my guess) is either calling std::isnan, which has the return type bool, or ROOT::Math::Internal::IsNan, which requires R__FAST_MATH and after pretending being smart and fast and whatever … converts an essentially bool expression into this Int_t for the reason that is unknown.
Maybe, they first were using isnan from C headers (which seems to be a macro there, since C does not have overloading and in my implementation, for example, resolves to something like
__header_always_inline int __inline_isnanf(float __x) {
// hehe, note, in C the result of != has type **int**
return __x != __x;
}
) and then somebody “modernized” the code by just prepending std:: and changing <math.h> to <cmath>.
Saying that - if you look at how this IsNan used, you’ll find cases there the type is, indeed, important - calling Max(isNan(), somethingElseNotBool) for example.
Thanks for the comprehensive review, @tpochep 
Still - this is hmm, awkward. Shall we fix TComplex to not use Max() but ||, and make IsNaN return a bool? @moneta - what do you think?
HI,
I think historically TMath::IsNaN was returning an int following the C insan implementation that returns an integer.
I agree we should change to Bool_t. It is not a backward-incompatible change, but it is clear in same cases users might get a compiler error, like when using it in template functions
Lorenzo
This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.