Menu
▾
▴
gnuplot-beta
|
From: Bastian M. <bma...@we...> - 2016-02-13 17:34:40
Attachments:
wxt_local_event.diff
|
Hi, Today I checked in some changes to the stable-5-0 branch which make gnuplot sources compatible with MSYS2/Mingw-w64 on Windows, as well as they make wxWidgets 3.0 work. These changes should in principle be compatible with other platforms, but I could only check on CentOS5 & 6. Of course it still compiles cleanly using MSVC2012 or the old MSYS/MinGW combination, too. There's another patch I would like to include, which avoids warnings about incompatible linkage of wxEvents on Windows. It works on CentOS6, too. Could someone please verify it does not break e.g. the Mac build? Bastian |
|
From: sfeam <sf...@us...> - 2016-02-13 20:24:08
|
On Saturday, 13 February 2016 06:34:27 PM Bastian Märkisch wrote:
> Hi,
>
> Today I checked in some changes to the stable-5-0 branch which make
> gnuplot sources compatible with MSYS2/Mingw-w64 on Windows, as well as
> they make wxWidgets 3.0 work. These changes should in principle be
> compatible with other platforms, but I could only check on CentOS5 & 6.
> Of course it still compiles cleanly using MSVC2012 or the old MSYS/MinGW
> combination, too.
In general it is not good to make changes in the stable branch that have not
already been tested in the main branch (5.1). Obviously there are exceptions
like fixing a bug that is only present in the stable branch.
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.
> There's another patch I would like to include, which avoids warnings
> about incompatible linkage of wxEvents on Windows. It works on CentOS6,
> too. Could someone please verify it does not break e.g. the Mac build?
It works for me on linux (tested on 2 systems), but again I'd be happier if it went
into 5.1 before applying it to the stable branch.
Ethan
> Bastian
|
|
From: Tatsuro M. <tma...@ya...> - 2016-02-13 21:50:13
|
----- Original Message ----- > From: sfeam > To: gnuplot-beta > Cc: Bastian Märkisch > Date: 2016/2/14, Sun 05:23 > Subject: Re: Changes to stable-5-0 branch > > On Saturday, 13 February 2016 06:34:27 PM Bastian Märkisch wrote: >> Hi, >> >> Today I checked in some changes to the stable-5-0 branch which make >> gnuplot sources compatible with MSYS2/Mingw-w64 on Windows, as well as >> they make wxWidgets 3.0 work. These changes should in principle be >> compatible with other platforms, but I could only check on CentOS5 & 6. >> Of course it still compiles cleanly using MSVC2012 or the old MSYS/MinGW >> combination, too. > > In general it is not good to make changes in the stable branch that have not > already been tested in the main branch (5.1). Obviously there are exceptions > like fixing a bug that is only present in the stable branch. > > 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. > I have been used this isnan change for MinGW-w64 (both 32 and 64bit) without any reports. Tatsuro |
|
From: Bastian M. <bma...@we...> - 2016-02-14 09:46:34
|
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.
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);
}
Bastian
|
|
From: sfeam <sf...@us...> - 2016-02-15 22:31:57
|
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
|
|
From: Hans-Bernhard B. <HBB...@t-...> - 2016-02-16 01:07:16
|
Am 15.02.2016 um 23:31 schrieb sfeam: > 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> > > and <math.h> as I understand it is required to define isnan() by C99. It would be. _But_ we're not running a C99 compiler here; we're running a C++ compiler. Now a C++11 compiler would be required (C++1x 26.8p3,p4) to effectively pull in the C99 version of <math.h>, including isnan(), except that in C++ it's not a macro: it's a set of three overloaded functions. C++ really doesn't want macros for this sort of thing. A compiler that's not running in C++11 mode (or equivalent) is not only not required to do that: it's even kind-of forbidden. Earlier editions of the C++ standard explicitly listed which functions <math.h> shall bring into a C++ program's global name space (C++03 26.5p1,p2). isnan() was not among them. To overcome this limit might take an equivalent of what the AC_USE_SYSTEM_EXTENSIONS method of autoconf does for us about the difference between "strict" and "extended" C, but for C++. > So why does the C++ compiler not pick up the C99 macro even if > it wouldn't otherwise define it on its own? Because the header itself hides it from C++'s view, probably by querying (__STDC_VERSION__ >= 199901L) and/or (__cplusplus >= 201103L) > If it's only this one place, may the simplest work-around is: > - if (isnan(*image)) > + if (*image != *image) // this test works even if isnan() is missing That might well be the case. |
