Ticket #10260 (closed defect: fixed)
module_list.py: quaternion_algebra_element.pyx wrongly uses -std=c99 option
| Reported by: | jdemeyer | Owned by: | GeorgSWeber |
|---|---|---|---|
| Priority: | major | Milestone: | sage-4.6.1 |
| Component: | build | Keywords: | |
| Cc: | Work issues: | ||
| Report Upstream: | N/A | Reviewers: | François Bissey |
| Authors: | Jeroen Demeyer | Merged in: | sage-4.6.1.alpha2 |
| Dependencies: | Stopgaps: |
Description
It's C++ code...
Attachments
Change History
comment:2 follow-up: ↓ 6 Changed 3 years ago by fbissey
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.
comment:3 follow-up: ↓ 4 Changed 3 years ago by 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.)
comment:4 in reply to: ↑ 3 Changed 3 years ago by fbissey
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.
comment:5 Changed 3 years ago by fbissey
- Status changed from needs_review to positive_review
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.
comment:6 in reply to: ↑ 2 Changed 3 years ago by jdemeyer
Replying to fbissey:
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.

