Bug 320803 - isnan() and isinf() should be std::isnan() and std::isinf()
Summary: isnan() and isinf() should be std::isnan() and std::isinf()
Status: RESOLVED FIXED
Alias: None
Product: kcalc
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: OpenBSD OpenBSD
: NOR normal
Target Milestone: ---
Assignee: Evan Teran
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-06 09:05 UTC by Vadim Zhukov
Modified: 2013-07-15 19:30 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Patch to fix issue (5.12 KB, patch)
2013-06-06 09:05 UTC, Vadim Zhukov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vadim Zhukov 2013-06-06 09:05:26 UTC
Since the <cmath> header is used, std::isnan() and std::isinf() should be used instead of C-like isnan() and isinf(). The former is guaranteed by standard, the latter is not - and, actually, it breaks on OpenBSD. The patch is attached.

Reproducible: Always
Comment 1 Vadim Zhukov 2013-06-06 09:05:53 UTC
Created attachment 80337 [details]
Patch to fix issue
Comment 2 Evan Teran 2013-06-06 13:53:07 UTC
This bug is actually a little more interesting than it looks. isnan and isinf are actually *not* part of the c++03 standard!

It is part of the c99 and c++11 standards.

In c99 is specifically defined as a macro.
In c++11 is described as being defined as both a macro and a function (the macro resolves to the function for C99 compatibility).

So for a non c++11 library implementation, it shouldn't  be in the std:: namespace at all.

kcalc will define macros if cmake fails to find these two functions, the question is, why is cmake finding them if they are only available in the std:: namespace on your platform? I don't think that your patch is the "right" solution because there certainly exist some other standard libraries which have isnan/isinf but don't put them in std:: because if the library is a conforming c++03 implementation, it shouldn't!

What version of OpenBSD are you running, I will set up a VM and investigate.
Comment 3 Vadim Zhukov 2013-06-06 21:07:53 UTC
General rule was always that all C stuff that's included from <c*> headers in C++ should be under std namespace. The KCalc compilation problems raised around 4.10.1, IIRC, some were fixed then and some were not.

CMake finds those because doesn't distinguish C and C++ in check_function_exists().

I'm working on OpenBSD-CURRENT. If you'll need any help on setup, feel free to contact me directly.
Comment 4 Evan Teran 2013-07-13 15:43:35 UTC
A fix has been posted to both Master and 4.11 branches. You should see it in the next release :-).
Comment 5 Raphael Kubo da Costa 2013-07-15 19:30:45 UTC
Git commit 6f8c665a1ac91d636d219d8b2e8fce28b3f22ad7 by Raphael Kubo da Costa.
Committed on 14/07/2013 at 22:07.
Pushed by rkcosta into branch 'KDE/4.11'.

Use math.h instead of cmath.

Also known as "let's try to prevent the #ifdef madness before it grows too
wild".

`isinf' and `isnan' are macros in C99, are not mentioned in C++03 and are
regular functions in C++11 (some C++ standard libraries such as GNU's
libstdc++ do implement those functions in C++98 and C++03 mode).

Simplify the current checks by unconditionally using math.h instead: this
allows us not to care about whether the `std' namespace is required and
assumes both `isinf' and `isnan' are macros, which are implemented when
undefined.
REVIEW:	111509

M  +0    -3    CMakeLists.txt
M  +2    -6    knumber/knumber_float.cpp

http://commits.kde.org/kcalc/6f8c665a1ac91d636d219d8b2e8fce28b3f22ad7