Re: Changes to stable-5-0 branch

[sfeam <sf...@us...>]

On Sunday, 14 February 2016 10:46:24 AM Bastian Märkisch wrote:
> 
> Am 13.02.2016 um 21:23 schrieb sfeam:
> (snip)
> > In particular the change below is problematic because it was already tried in
> > the main branch and turned out to cause problems on some systems:
> >
> >          * src/qtterminal/qt_term.h src/qtterminal/qt_conversion.cpp:  isnan()
> >          is in namespace std.
> >
> > See 5.1 ChangeLog:
> >
> > 2015-12-10  Hans-Bernhard Broeker  <br...@ph...>
> >
> >          * src/qtterminal/qt_conversion.cpp (qt_imageToQImage): Do not call
> >          C++ isnan() without a namespace specifier.
> >
> >          EAM:  Reverting this change.  We may need a fix, but this isn't it.
> >          qtterminal/qt_conversion.cpp:
> >          In function 'QImage qt_imageToQImage(int, int, coordval*, t_imagecolor)':
> >          qtterminal/qt_conversion.cpp:129:14:
> >          error: expected unqualified-id before '(' token if (std::isnan(*image))
> >
> > I don't know what the issue is here, but since adding the std:: qualifier
> > is known to break the build on some systems that were perfectly happy before
> > this, I think it is not suitable for the stable branch.
> >
> 
> OK. Reverted in branch-5-0-stable.
> 
> The problem is that isnan is in the C99 standard, but wasn't in the C++ 
> standard until C++11. Hence, many C++ compilers implemented it "their" 
> way, as _isnan (MSVC), ::isnan, std::isnan or macro (as in C99). See 
> e.g. the discussion at
> http://stackoverflow.com/questions/570669/checking-if-a-double-or-float-is-nan-in-c
> Btw. simply adding a "using namespace std" doesn't solve the issue 
> either, since some compilers have both, ::isnan and std::isnan.

I am having trouble to understand how in our particular case C++ can
fail to find the C language definition of isnan() from <math.h>

The source line at issue is 

qt_conversion.cpp:129:                          if (isnan(*image))
included from 
qt_term.cpp:78:#include "qt_conversion.cpp"

But before this we have
qt_term.cpp:57: #include "term_api.h"  // for stdfn.h, JUSTIFY, encoding, ...
term_api.h:43:#include "stdfn.h"
stdfn.h:291:# include <math.h>

and <math.h> as I understand it is required to define isnan() by C99.
So why does the C++ compiler not pick up the C99 macro even if
it wouldn't otherwise define it on its own?

> 
> Too me it looks like the only way of solving this in a portable way is 
> to implement a C function like this in e.g. stdfn.c|h :
> 
> TBOOLEAN gp_isnan(double v)
> {
> 	return isnan(v);
> }

If it's only this one place, may the simplest work-around is:

--- old/qt_conversion.cpp  2016-02-14 13:51:54.866487634 -0800
+++ new/qt_conversion.cpp     2016-02-15 14:29:15.785792815 -0800
@@ -126,7 +126,7 @@ QImage qt_imageToQImage(int M, int N, co
                        QRgb* line = (QRgb*)(qimage.scanLine(n));
                        for (int m = 0; m < M; m++)
                        {
-                               if (isnan(*image))
+                               if (*image != *image)   // this test works even if isnan() is missing
                                {
                                        image++;
                                        *line++ = 0x00000000;



	Ethan

> 
>    Bastian