module_list.py: quaternion_algebra_element.pyx wrongly uses -std=c99 option

[jdemeyer]

It's C++ code...

Component: build

Author: Jeroen Demeyer

Reviewer: François Bissey

Merged: sage-4.6.1.alpha2

Issue created by migration from https://trac.sagemath.org/ticket/10260

[jdemeyer]

comment:1

Attachment: 10260_quaternion_algebra_element_cpp.patch.gz

[kiwifb]

comment:2

Tell you what. I introduced exactly the same patch in sage-on-gentoo at the end of June. While you are at it can we fix this one as well:

    Extension('sage.rings.polynomial.polynomial_rational_flint',
              sources = ['sage/rings/polynomial/polynomial_rational_flint.pyx', 'sage/libs/flint/fmpq_poly.c'],
              language = 'c++',
              extra_compile_args=["-std=c99"] + uname_specific('SunOS', [], ['-D_XPG6']),
              libraries = ["csage", "flint", "ntl", "gmpxx", "gmp"],
              include_dirs = [SAGE_ROOT + '/local/include/FLINT/', SAGE_ROOT + '/devel/sage/sage/libs/flint/'],
              depends = [SAGE_ROOT + "/local/include/FLINT/flint.h"]),

That's the only other with that "QA" problem. I'll happily give it a positive review.

[robertwb]

comment:3

Could you explain what "D_XPG6" is and why it's needed? (I've seen this elsewhere, and yet to have anyone justify it.)

[kiwifb]

comment:4

Replying to @robertwb:

Could you explain what "D_XPG6" is and why it's needed? (I've seen this elsewhere, and yet to have anyone justify it.)

After a quick googling it looks like something ugly. First hit says:
XPG was the X/Open Portability Guide, a pre-POSIX consortium to promote interoperability between different UNIX vendors. XPG6 is "X/Open Portability Guide, Issue 6".

But that one is a little bit more verbose about what that flag is supposed to do:
http://unix.derkeiler.com/Newsgroups/comp.unix.solaris/2007-01/msg00360.html
In short make C99 code work inside c++ code... Which may explain why someone
bothered to put it together with -std=c99. Also the post ask how to enable it
on solaris which explains why there is a uname_specific command with it.

It seem to be regarded as a bad idea by a number of people.

[kiwifb]

comment:5

I am giving the present patch a positive review. It is fairly simple and as actually been in use in sage-on-gentoo for a while.

We can do the other one later in another ticket.

[jdemeyer]

comment:6

Replying to @kiwifb:

Tell you what. I introduced exactly the same patch in sage-on-gentoo at the end of June. While you are at it can we fix this one as well:

    Extension('sage.rings.polynomial.polynomial_rational_flint',
              sources = ['sage/rings/polynomial/polynomial_rational_flint.pyx', 'sage/libs/flint/fmpq_poly.c'],
              language = 'c++',
              extra_compile_args=["-std=c99"] + uname_specific('SunOS', [], ['-D_XPG6']),
              libraries = ["csage", "flint", "ntl", "gmpxx", "gmp"],
              include_dirs = [SAGE_ROOT + '/local/include/FLINT/', SAGE_ROOT + '/devel/sage/sage/libs/flint/'],
              depends = [SAGE_ROOT + "/local/include/FLINT/flint.h"]),

This one is harder, because there are two files, one C99 and one C++. So we cannot simply remove -std=c99 there.

[jdemeyer]

Reviewer: François Bissey

[jdemeyer]

Merged: sage-4.6.1.alpha2