On Thu, Oct 20, 2011 at 05:08:50PM -0700, Alan Irwin wrote:
> On 2011-10-20 15:23+0100 Andrew Ross wrote:
> 
> > Gcc with no standards flags is now warning free again.
> 
> Thanks, Andrew, for those changes which did solve the broken build for
> 
> export CFLAGS='-O3 -fvisibility=hidden'
> export CXXFLAGS='-O3 -fvisibility=hidden'
> export FFLAGS='-O3'
> 
> on my platform.  Furthermore, "ctest -j8" completed with no errors.
> 
> However, there are 3 similar build warnings still left on my platform which 
> are
> as follows:
> 
> In file included from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/qt.h:72,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/examples/c++/qt_PlotWindow.h:39,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/examples/c++/qt_example.cpp:26:
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/plplotP.h:273:1: 
> warning: "isfinite" redefined
> In file included from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/plplotP.h:111,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/qt.h:72,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/examples/c++/qt_PlotWindow.h:39,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/examples/c++/qt_example.cpp:26:
> /usr/include/math.h:245:1: warning: this is the location of the previous 
> definition
> 
> In file included from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/qt.h:72,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/examples/c++/qt_PlotWindow.h:39,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/examples/c++/qt_PlotWindow.cpp:26:
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/plplotP.h:273:1: 
> warning: "isfinite" redefined
> In file included from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/plplotP.h:111,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/qt.h:72,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/examples/c++/qt_PlotWindow.h:39,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/examples/c++/qt_PlotWindow.cpp:26:
> /usr/include/math.h:245:1: warning: this is the location of the previous 
> definition
> 
> In file included from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/qt.h:72,
>                   from 
> /home/software/plplot_svn/HEAD/build_dir/examples/c++/../../../plplot_cmake_qt/examples/c++/qt_PlotWindow.h:39,
>                   from 
> /home/software/plplot_svn/HEAD/build_dir/examples/c++/moc_qt_PlotWindow.cxx:10:
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/plplotP.h:273:1: 
> warning: "isfinite" redefined
> In file included from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/plplotP.h:111,
>                   from 
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/include/qt.h:72,
>                   from 
> /home/software/plplot_svn/HEAD/build_dir/examples/c++/../../../plplot_cmake_qt/examples/c++/qt_PlotWindow.h:39,
>                   from 
> /home/software/plplot_svn/HEAD/build_dir/examples/c++/moc_qt_PlotWindow.cxx:10:
> /usr/include/math.h:245:1: warning: this is the location of the previous 
> definition
> 
> The relevant lines from math.h on my platform are as follows:
> 
> /* Return nonzero value if X is not +-Inf or NaN.  */
> # ifdef __NO_LONG_DOUBLE_MATH
> #  define isfinite(x) \
>       (sizeof (x) == sizeof (float) ? __finitef (x) : __finite (x))
> # else
> #  define isfinite(x) \
>       (sizeof (x) == sizeof (float)                                           
>  \
>        ? __finitef (x)                                                        
>  \
>        : sizeof (x) == sizeof (double)                                        
>  \
>        ? __finite (x) : __finitel (x))
> # endif
> 
> line 245 is just after the #else so apparently my system does have long
> double math.

I'd independently spotted this doing the same test as you.

The problem is that the "internal" plplotP.h header is actually used 
indirectly in the qt examples. In this case HAVE_CONFIG_H is not set to 
some of the defines (e.g. the check for isfinite) are missing. It is 
similar to the problem I fixed yesteday with ocamlc not setting 
HAVE_CONFIG_H. I've moved the required defines to plConfig.h to solve the 
problem. It would be nice though if plplotP.h was truly internal. In my 
opinion this should just be used for the build. It is not part of the API 
and so ideally we shouldn't need to expose the user to it or install it.

I've done some further testing on another system with a newer version of
gcc and this has exposed some further warnings. In particular it is 
now much more picky about const for 2-d arrays, i.e. pointers to pointers
The problem is that we make use of things like const PLFLT **a. The says 
that a is a pointer to an array of pointers which in turn point to arrays
of PLFLT. The const says that the arrays of PLFLT are constant, but NOT
that the array of pointers is constant. It is therefore trivial to 
change the pointers in order to change the underlying data. This undermines
the const guarantee. Strictly one would usually want to do something like
const PLFLT * const *a which guarantees that both the PLFLT arrays and
the PLFLT * array are constant. This leads to quite ugly definitions though.
Invariably const specifiers in C are quite pervasive. Once you have made
something const it has to propogate through subsequent function calls. 

Fixing these warnings is likely to result in API changes to the contouring
/ shading functions which use const PLFLT **. The code ugliness and 
clarity could probably be addressed using some typedefs. E.g.
something like

typedef (const PLFLT * const *) PLFLT_array2D_const;

The real question is, "Is this worth it for a potentially
pervasive API change just to fix compiler warnings?"

Another issue I've encountered is that PLplot makes a fair bit of use
of casting function pointers to PLpointer (i.e. void *) for passing
between functions. This contravenes ISO C as there is no 
guarantee that function pointers and object pointers have the same
size. In practice they do, which is why the code works, but we might
want to consider a cleaner way of doing this. Again this is a potential
API change, but it is a clear contravention of standards as opposed to 
just a compiler warning.

I have had no response to my previous email about handling unused parameter
values. I take that as assent to my proposals, so I will try implementing 
the UNUSED macro approach, at least for the core library code in src/ .
Once there is a concrete implementation in place people might be in a
better position to comment.

I realise this is a lot of questions, but given that fixing these
issues might involve API changes I wanted to seek some sort of
consensus, or at least give people the chance to comment, before
making any changes.

Andrew

------------------------------------------------------------------------------
The demand for IT networking professionals continues to grow, and the
demand for specialized networking skills is growing even more rapidly.
Take a complimentary Learning@Cisco Self-Assessment and learn 
about Cisco certifications, training, and career opportunities. 
http://p.sf.net/sfu/cisco-dev2dev
_______________________________________________
Plplot-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/plplot-devel

Reply via email to