port lcalc to C++11

[dimpase]

lcalc doesn't use standard C++:

• using namespace std; needs an include with stuff from std:: before it

• typeof is a gcc extension - C++11 has decltype instead

• plain C must not be compiled with C++ compiler - thus a makefile change

[fbissey]

Can you rebase on 8.0.rc0? Your branch shows up in red.

[dimpase]

this is needed for building lcalc with -std=c++11. (tested so far with clang 4.0.1 on Linux, with stdlib=libc++)

Only the top commit is of interest. Perhaps it can be based on rc0 indeed. But I need sleep now :-)

[fbissey]

OK :)

The next question is how to ensure that the compiler then uses c++11. lcalc doesn't have a configure script so we cannot do it there. At the very least spkg-install will need to insert -std=c++11 in CXXFLAGS to make sure you are always using c++11.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​5434c70	Merge branch 'develop' into compiler
​f565739	bump config tarball
​53f9c4d	Merge branch 'compiler' into clang-progress
​7a9ab6b	checksum for this branch's configure tarball
​14e08ae	Merge branch 'u/fbissey/clang-progress' of trac.sagemath.org:sage into clang

[dimpase]

the other problem here is an incorrect building of lcalc extension. Here C++ is compiled with a C compiler, which is patently wrong. Any idea how this can be fixed?

[sagelib-8.0.rc0] [  4/323] clang -fno-strict-aliasing -OPT:Olimit=0 -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -fPIC -I/mnt/opt/Sage/sage-clang/local/lib/python2.7/site-packages/cypari2 -I/mnt/opt/Sage/sage-clang/local/lib/python2.7/site-packages/cysignals -I/mnt/opt/Sage/sage-clang/local/include -I/mnt/opt/Sage/sage-clang/local/include/python2.7 -I/mnt/opt/Sage/sage-clang/local/lib/python2.7/site-packages/numpy/core/include -I/mnt/opt/Sage/sage-clang/src -I/mnt/opt/Sage/sage-clang/src/sage/ext -I/mnt/opt/Sage/sage-clang/src/build/cythonized -I/mnt/opt/Sage/sage-clang/src/build/cythonized/sage/ext -I/mnt/opt/Sage/sage-clang/local/include/python2.7 -c /mnt/opt/Sage/sage-clang/src/build/cythonized/sage/libs/lcalc/lcalc_Lfunction.cpp -o build/temp.linux-x86_64-2.7/mnt/opt/Sage/sage-clang/src/build/cythonized/sage/libs/lcalc/lcalc_Lfunction.o -fno-strict-aliasing -O3 -ffast-math
[sagelib-8.0.rc0] In file included from /mnt/opt/Sage/sage-clang/src/build/cythonized/sage/libs/lcalc/lcalc_Lfunction.cpp:505:
[sagelib-8.0.rc0] In file included from /mnt/opt/Sage/sage-clang/src/sage/libs/lcalc/lcalc_sage.h:1:
[sagelib-8.0.rc0] In file included from /mnt/opt/Sage/sage-clang/local/include/libLfunction/L.h:34:
[sagelib-8.0.rc0] In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/include/g++-v6/backward/strstream:50:
[sagelib-8.0.rc0] /usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/include/g++-v6/backward/backward_warning.h:32:2: warning: This file includes at least one deprecated or antiquated header which   may be removed without further notice at a future date. Please use a   non-deprecated interface with equivalent functionality instead. For a   listing of replacement headers and interfaces, consult the file   backward_warning.h. To disable this warning use -Wno-deprecated. [-W#warnings]
[sagelib-8.0.rc0] #warning \
[sagelib-8.0.rc0]  ^
[sagelib-8.0.rc0] In file included from /mnt/opt/Sage/sage-clang/src/build/cythonized/sage/libs/lcalc/lcalc_Lfunction.cpp:505:
[sagelib-8.0.rc0] In file included from /mnt/opt/Sage/sage-clang/src/sage/libs/lcalc/lcalc_sage.h:1:
[sagelib-8.0.rc0] In file included from /mnt/opt/Sage/sage-clang/local/include/libLfunction/L.h:40:
[sagelib-8.0.rc0] In file included from /mnt/opt/Sage/sage-clang/local/include/libLfunction/Lglobals.h:48:
[sagelib-8.0.rc0] /mnt/opt/Sage/sage-clang/local/include/libLfunction/Lcommon.h:71:8: error: expected ';' in 'for' statement specifier
[sagelib-8.0.rc0]                 loop(i,this->N,N) coeffs[i]=zero;
[sagelib-8.0.rc0                       ^
...

[dimpase]

Replying to fbissey:

OK :)

The next question is how to ensure that the compiler then uses c++11. lcalc doesn't have a configure script so we cannot do it there. At the very least spkg-install will need to insert -std=c++11 in CXXFLAGS to make sure you are always using c++11.

We may assume that all the C++ compilers we care about support -std=c++11, right? So this may be an independent of clang tickets thing?

[dimpase]

Jeroen, do you know to force the use of C++ compiler here (with proper use of CXXFLAGS, too) ?

[fbissey]

Replying to dimpase:

the other problem here is an incorrect building of lcalc extension. Here C++ is compiled with a C compiler, which is patently wrong. Any idea how this can be fixed?

Yes. This is ​https://bugs.python.org/issue1222585 the patch needed for that is included in the gentoo patch tarball for python.

[fbissey]

Replying to dimpase:

We may assume that all the C++ compilers we care about support -std=c++11, right? So this may be an independent of clang tickets thing?

Definitely. This shouldn't depend on #12426.

[dimpase]

Replying to fbissey:

Replying to dimpase:

the other problem here is an incorrect building of lcalc extension. Here C++ is compiled with a C compiler, which is patently wrong. Any idea how this can be fixed?

Yes. This is ​https://bugs.python.org/issue1222585 the patch needed for that is included in the gentoo patch tarball for python.

Is there a Sage ticket for this already?

[fbissey]

Replying to dimpase:

Replying to fbissey:

Replying to dimpase:

the other problem here is an incorrect building of lcalc extension. Here C++ is compiled with a C compiler, which is patently wrong. Any idea how this can be fixed?

Yes. This is ​https://bugs.python.org/issue1222585 the patch needed for that is included in the gentoo patch tarball for python.

Is there a Sage ticket for this already?

I don't think so. But it comes up regularly.

[jdemeyer]

How urgent is this? It might be possible to drop the lcalc package completely in favour of PARI/GP. However, the timeframe for this might be in the order on months (mainly because I don't know enough mathematically about L-functions).

[fbissey]

Replying to jdemeyer:

How urgent is this? It might be possible to drop the lcalc package completely in favour of PARI/GP. However, the timeframe for this might be in the order on months (mainly because I don't know enough mathematically about L-functions).

I think Dima started this with the view of compiling sage with -std=c++11. If that's what he is doing, getting ​https://bugs.python.org/issue1222585 in would be more pressing in my opinion.

[dimpase]

Replying to fbissey:

Replying to jdemeyer:

How urgent is this? It might be possible to drop the lcalc package completely in favour of PARI/GP. However, the timeframe for this might be in the order on months (mainly because I don't know enough mathematically about L-functions).

I think Dima started this with the view of compiling sage with -std=c++11.

indeed, and as a part of the clang porting effort too - it's just too much trouble to have different packages with different c++xy IMHO, especially if one depends on the another, they better be on the same xy...

If that's what he is doing, getting ​https://bugs.python.org/issue1222585 in would be more pressing in my opinion.

I'll see how it flies with these patches, and open a ticket.

[fbissey]

Replying to dimpase:

I'll see how it flies with these patches, and open a ticket.

For info Gentoo's python includes those. sage-on-gentoo has been compiled using these patches to distutils for almost as long as it existed.

[dimpase]

For the life of me, I don't understand why the patch ​21_all_distutils_c++.patch from ​https://dev.gentoo.org/~floppym/python/python-gentoo-patches-2.7.13-0.tar.xz does not apply if I place it in build/pkgs/python2/patches/: all the files etc are as they should be, yet I get

[python2-2.7.13.p1] Applying patches from ../patches...
[python2-2.7.13.p1] Applying ../patches/2.6.5-FD_SETSIZE.patch
[python2-2.7.13.p1] patching file Modules/selectmodule.c
[python2-2.7.13.p1] Applying ../patches/21_all_distutils_c++.patch
[python2-2.7.13.p1] can't find file to patch at input line 5
[python2-2.7.13.p1] Perhaps you used the wrong -p or --strip option?
[python2-2.7.13.p1] The text leading up to this was:
[python2-2.7.13.p1] --------------------------
[python2-2.7.13.p1] |https://bugs.python.org/issue1222585
[python2-2.7.13.p1] |
[python2-2.7.13.p1] |--- Lib/_osx_support.py
[python2-2.7.13.p1] |+++ Lib/_osx_support.py
[python2-2.7.13.p1] --------------------------
[python2-2.7.13.p1] File to patch: 
...

Seems to be the right format, the right paths to files, yet this...

[fbissey]

Check the patch directory levels with another patch there may be one directory to many in the Gentoo patch compared to what sage's automatic patching needs.

[dimpase]

Replying to fbissey:

Check the patch directory levels with another patch there may be one directory to many in the Gentoo patch compared to what sage's automatic patching needs.

no, it's not the case as far as I can see (and I've been stuck with it for an hour already, just not yet ready to apply it by hand...)

[fbissey]

Just checked you need to _add_ one directory level to the gentoo patch. That is

--- Lib/_osx_support.py
+++ Lib/_osx_support.py

needs to become

--- a/Lib/_osx_support.py
+++ b/Lib/_osx_support.py

and so on...

[dimpase]

Replying to fbissey:

Just checked you need to _add_ one directory level to the gentoo patch. That is

--- Lib/_osx_support.py
+++ Lib/_osx_support.py

needs to become

--- a/Lib/_osx_support.py
+++ b/Lib/_osx_support.py

and so on...

Oh, thanks, I've been blind...

[dimpase]

Replying to fbissey:

OK :)

The next question is how to ensure that the compiler then uses c++11. lcalc doesn't have a configure script so we cannot do it there. At the very least spkg-install will need to insert -std=c++11 in CXXFLAGS to make sure you are always using c++11.

Somehow I cannot manage to achieve this; it all works if I do

CXXFLAGS="-std=c++11" ./sage -f lcalc

but somehow does not work if I put into spkg-install

$MAKE CXXFLAGS="-std=c++11"

some evil Makefile magic that escapes me here. :-(

[fbissey]

Most other spkg-install do something like

CXXFLAGS="-std=c++11 ${CXXFLAGS}"
export CXXFLAGS
$MAKE

[dimpase]

Replying to fbissey:

Most other spkg-install do something like

CXXFLAGS="-std=c++11 ${CXXFLAGS}"
export CXXFLAGS
$MAKE

no difference, it still does not work. It might be due to the Makefile modifying CXXFLAGS: it does

CXXFLAGS := -O3 $(OPENMP_FLAG) $(PREPROCESSOR_DEFINE) $(MACHINE_SPECIFIC_FLAGS) $(EXTRA) $(CXXFLAGS)

[dimpase]

I don't know:

• bash bug? (version 4.4.12(1))
• make bug? (4.2.1)

will try on something older...

[fbissey]

logs please.

[rws]

I'm willing to do some bulk C++11 work, as I did with Pynac. Is there a public lcalc repo?

[dimpase]

this is the branch, as here are the results of running

./sage -f lcalc

​http://users.ox.ac.uk/~coml0531/sage/lcalc-1.23.p16.log

vs the result of running

CXXFLAGS="-std=c++11" ./sage -f lcalc

​http://users.ox.ac.uk/~coml0531/sage/externalCXXFLAGS_lcalc-1.23.p16.log

As you see in the 1st log CXXFLAGS are ignored. Tested on Fedora25 as well as on Gentoo. What am I missing?!!

PS. F25 bash is 4.3.43(1)-release, and make is 4.1.

[dimpase]

Replying to rws:

I'm willing to do some bulk C++11 work, as I did with Pynac. Is there a public lcalc repo?

It's done here, the problem which remains is to fight the makefile madness

There will be more similar C++ "fun" for gfan (I have a patch, but it produces lots of test errors), giac... I'll cc you on these tickets as they appear.

[fbissey]

Replying to rws:

I'm willing to do some bulk C++11 work, as I did with Pynac. Is there a public lcalc repo?

No.

[dimpase]

let us make one on github.

[rws]

Replying to dimpase:

There will be more similar C++ "fun" for gfan (I have a patch, but it produces lots of test errors), giac... I'll cc you on these tickets as they appear.

Note BTW there is gfan-0.6 now.

[dimpase]

in fact, a lcalc repo is here : ​https://code.google.com/archive/p/l-calc/

last commit is from 2014, but it's newer than what we use, see ​http://oto.math.uwaterloo.ca/~mrubinst/L_function_public/CODE/

[rws]

I just did one for gfan: ​https://github.com/rwst/gfan06

[fbissey]

This is quite peculiar. Looking more closely, even the section for OS X in spkg-install, doesn't work properly

if [ "$UNAME" = "Darwin" ]; then
    export LDFLAGS="$LDFLAGS -Wl,-headerpad_max_install_names"
fi

we still get the right thing because there is darwin section in the makefile but the export above doesn't work.

[dimpase]

Replying to dimpase:

in fact, a lcalc repo is here : ​https://code.google.com/archive/p/l-calc/

last commit is from 2014, but it's newer than what we use, see ​http://oto.math.uwaterloo.ca/~mrubinst/L_function_public/CODE/

I've imported the code.google (hg) repo into git, here: ​https://github.com/dimpase/lcalc

I'll try merging Sage's changes there.

[rws]

That repo code is different enough and clang-check gives lots of errors. If we do this together, how to coordinate?

[dimpase]

Replying to rws:

That repo code is different enough and clang-check gives lots of errors. If we do this together, how to coordinate?

If you like to take the lead on this, please go ahead---I have my hands full with other things.

[rws]

Replying to dimpase:

If you like to take the lead on this, please go ahead---I have my hands full with other things.

NP. Actual code changes will go in a PR.

[rws]

Replying to rws:

NP. Actual code changes will go in a PR.

​https://github.com/dimpase/lcalc/pull/1

Please review. I haven't worked on upgrading the Sage package, nor have I tested functionality.

[jdemeyer]

Two questions about this ticket:

1. Is it about lcalc itself, Sage modules using lcalc, or both?

2. Instead of porting lcalc to C++11, is it an option to force -std=c++98 instead?

[jdemeyer]

Hmm, maybe it's not a matter of porting to C++11, but just porting to proper C++ instead. At least src/sage/libs/lcalc/lcalc_Lfunction.pyx doesn't compile with -std=c++98 either: it requires either -std=gnu++98 or -std=gnu++11.

[jdemeyer]

Replying to fbissey:

I think Dima started this with the view of compiling sage with -std=c++11. If that's what he is doing, getting ​https://bugs.python.org/issue1222585 in would be more pressing in my opinion.

I created #7028 for that.

[dimpase]

There is one more thing to add here; there is a macro double(x) defined in include/Lcommon.h, which does not seem to be used anywhere, and which causes havoc sometimes, e.g. if I try building with gcc on FreeBSD (due to cephes present)

Anyway, hey, redefining a language keyword, yuck...

[rws]

Because of what I logged in my comments in #23898 it looks like current lcalc does not compile on OpenSuSE systems with system gcc-5.3.1. A patch is at ​https://trac.sagemath.org/ticket/23898#comment:39.

[rws]

Replying to dimpase:

There is one more thing to add here; there is a macro double(x) defined in include/Lcommon.h, which does not seem to be used anywhere, and which causes havoc sometimes, e.g. if I try building with gcc on FreeBSD (due to cephes present)

This is outcommented in the github lcalc version which also, with the above PR, passes clang-check. It looks like an improvement. OTOH this ticket seems no longer pressing, as the Sage version compiles with clang too.

The attempt to replace Sage's lcalc with Pari (#24532) was dropped, so maybe it's an idea to at least upgrade it to the github version?

[dimpase]

I am all for it, but I'm a bit lost here. What stage are we at?

[rws]

Replying to dimpase:

What stage are we at?

This is the first ticket that mentions the 2014 version at all I think.

[rws]

There is mention in ​https://trac.sagemath.org/ticket/11321#comment:8 but this is only because patches were sent upstream.

[rws]

The github version defines itself in CHANGE.LOG and include/cmdline.h as v. 1.3, so we might stick to that and add something. I mean with that the name of the tarball later distributed, not our Sage patchlevel. I personally prefer ISO dates like 20180223 whenever possible.

[jdemeyer]

Replying to rws:

I personally prefer ISO dates like 20180223 whenever possible.

What does the date refer to? If you are packaging something which does not have a naturally defined date, I wouldn't use a date. For packages created from a git repo, better use a git commit hash.

[rws]

Replying to jdemeyer:

For packages created from a git repo, better use a git commit hash.

That's fine too.

[rws]

The changes to install the package without patches are now added to ​https://github.com/dimpase/lcalc/pull/1 which eventually needs a review. ATM Sage does NOT build with it as compilation of the Cython module fails.

One problem worked around was that the use of FP type (double,mpfr,etc..) can be given to make but is not fixed in a header like with conf.h in other packages. I have hardcoded the type used until now (double) but this is an issue.

[dimpase]

I've just looked at the pull request again - do you see my comments there?

[rws]

Replying to dimpase:

I've just looked at the pull request again - do you see my comments there?

Yes, I have answered there.

[rws]

I opened #24280 with the changes on the Sage side. Testing yields mostly tolerance exceeded but also some different values that need a look.

[dimpase]

Replying to rws:

I opened #24280 with the changes on the Sage side. Testing yields mostly tolerance exceeded but also some different values that need a look.

seems to be wrong reference, what is the correct ticket?

[rws]

Please see #24820.

[gh-timokau]

The ntl upgrade (#25532) depends on this. Is there a reason this (or #24820) isn't needs_review?

[rws]

Replying to gh-timokau:

The ntl upgrade (#25532) depends on this. Is there a reason this (or #24820) isn't needs_review?

I think #24820 is the newer attempt. It would be necessary to agree that either the Hardy-Z function is not defined for complex argument or lcalc is unable to compute the case. As I said in #24820, I have no idea about the usefulness of complex argument results. Do you?

[jdemeyer]

Replying to gh-timokau:

The ntl upgrade (#25532) depends on this.

As I said on that ticket, the dependency itself could be considered a bug. It is probably easier to remove the dependency than fixing lcalc.

[gh-timokau]

Replying to rws:

Replying to gh-timokau:

The ntl upgrade (#25532) depends on this. Is there a reason this (or #24820) isn't needs_review?

I think #24820 is the newer attempt. It would be necessary to agree that either the Hardy-Z function is not defined for complex argument or lcalc is unable to compute the case. As I said in #24820, I have no idea about the usefulness of complex argument results. Do you?

Unfortunately I don't :/

Replying to jdemeyer:

Replying to gh-timokau:

The ntl upgrade (#25532) depends on this.

As I said on that ticket, the dependency itself could be considered a bug. It is probably easier to remove the dependency than fixing lcalc.

At least that would fix #25532. Isn't the dependency there for a reason?

[fbissey]

I think it is time we move forward with this again. I think this ticket should also include the matching patch from arch in sagelib ​https://git.archlinux.org/svntogit/community.git/plain/trunk/sagemath-lcalc-c++11.patch?h=packages/sagemath&id=0e31ae526ab7c6b5c0bfacb3f8b1c4fd490035aa so that we really are just using C++11 and allow us to make progress on #25532.

Any objections to moving forward with what we have now?

[fbissey]

Yes this need the extra patch from arch I quoted. Otherwise sage's compilation fails because lcalc is no using construct that don't exist in gnu++98.

[fbissey]

Last call for patches. Please someone review. It would be good to have this in the next beta and be able to move on to #25532 and get it all done for 8.5.

---

New commits:

​108d581	Merge branch 'develop' into lcalc_cxx11
​834e7a6	lcalc version bump for rebuild
​837732e	remove enforcement of gnu++98 in lcalc bindings

[fbissey]

Anyone interested in reviewing this?

[jdemeyer]

1. Can you add some rationale for the changes to Makefile?

2. I wouldn't make style changes such as

   -               loop(i,this->N,N)
   -                       coeffs[i]=zero;
   +               loop(i,this->N,N) coeffs[i]=zero;
   

[jdemeyer]

Replying to fbissey:

It would be good to have this in the next beta and be able to move on to #25532

As I mentioned multiple times, I don't really think that #25532 must depend on this.

[fbissey]

Replying to jdemeyer:

Replying to fbissey:

It would be good to have this in the next beta and be able to move on to #25532

As I mentioned multiple times, I don't really think that #25532 must depend on this.

Well, I tried to see if I could up ntl separately in sage-on-gentoo and sage wouldn't compile. It could be that I can tweak ntl's configuration to be compatible (at possible cost in ntl internal) but I'd rather get rid of the last bit of sage that is c++11 resistant (I think) and go with the natural flow again.

[jdemeyer]

Minor nitpick: wouldn't it be cleaner to replace decltype(m) i=(m) by auto i = (m)?

[fbissey]

Replying to jdemeyer:

Minor nitpick: wouldn't it be cleaner to replace decltype(m) i=(m) by auto i = (m)?

It has indeed a better feel. A bit too late in the day for me to do any changes (safely), I'll go through your reviews in the morning.

[jdemeyer]

Replying to fbissey:

A bit too late in the day for me

You chose to live on the wrong side of the world :-)

[fbissey]

Replying to jdemeyer:

Replying to fbissey:

A bit too late in the day for me

You chose to live on the wrong side of the world :-)

Well it feels like it will be a nice summer. And not too hot, like our australian neighbors :-) but I really need to get some sleep now.

[jdemeyer]

In your Makefile patch, the rule $(CC) $(CFLAGS) $(INCLUDEFILES) -c cmdline.c is just the standard build rule for .c files (modulo the typo $(CFLAGS)->$(CCFLAGS)). So it might be better to just restore the comment.

[fbissey]

OK some comments on the review from Jeroen which made me dig a bit deeper on the Makefile. I must say I adopted a slightly different approach in sage-on-gentoo so I don't have the same patch, but some of lesson learnt will definitely apply.

• the original makefile defines CC as as g++ so that has to go.
• Non standard flags like CCFLAGS are used, so Dima tried to use appropriate flags for both C and C++ compilers. And it also differentiate C and C++ flags which wasn't done before because even regular C code was passed to the C++ compiler.
• The downside of using CFLAGS and CXXFLAGS inside the makefile is that regular options passed from the sage build system may need care if we want to inject them. That's a sage-on-gentoo difference I use MYCXXFLAGS so I can include whatever other flags we pass without worry.
• Jeroen points out the standard rule for .c file should be used for cmdline.c except that the current patch erroneously define the rule as using CXXFLAGS instead of CFLAGS. Oopsie.

Point of difference with sage-on-gentoo: because pari headers are included in two files with #include "pari.h" the makefile has to define the location of pari headers and so do any code that include these headers. Decided that it was not worth it and patched to have #include "pari/pari.h" which is more sane and means that we don't have to worry about adding non standard includes. That means that I cut an extra bit of the makefile too. If people think that's a good idea I'll include that change in sage too.

[jdemeyer]

Replying to fbissey:

because pari headers are included in two files with #include "pari.h" the makefile has to define the location of pari headers and so do any code that include these headers. Decided that it was not worth it and patched to have #include "pari/pari.h" which is more sane and means that we don't have to worry about adding non standard includes. That means that I cut an extra bit of the makefile too. If people think that's a good idea I'll include that change in sage too.

That makes sense to me.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​8190cef	Correct inclusion of pari header so no extra -I flags are required.
​71f8bf2	update makefile patch and document it
​eb072d1	update the patch according to Jeroen's review. Add a correction of a "std:" which should have been "std::"

[fbissey]

All right back to review.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​3fb7f80

std: was introduced in the clang patch. Correcting in that patch.

[fbissey]

So many patches! It is hard to figure if the thing you patch was introduced by another patch :P

[dimpase]

This appears to work with clang 6.0.1, a good sign.

[fbissey]

Replying to dimpase:

This appears to work with clang 6.0.1, a good sign.

I have checked clang-6 and gcc-8.2. Now if someone would put a kind review.

Although Dima, I kept the patch name "using_namespace_std.patch` but there is hardly any of that in there. Just removing GNU-ism. Did you have more stuff to do in mind?

[dimpase]

OK

[gh-timokau]

Thank you @fbissey for pushing this forward, happy to have one less patch to apply :)

[vbraun]

On some machines this fails with:

make libLfunction.so
make[5]: Entering directory '/home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/lcalc-1.23.p17/src/src'
g++ -O3   -ffast-math -fPIC   -I../include -c Lglobals.cc
In file included from ../include/Lglobals.h:48:0,
                 from Lglobals.cc:23:
../include/Lcommon.h: In member function 'void smallPoly<T>::resize(int)':
../include/Lcommon.h:71:8: error: 'i' does not name a type
   loop(i,this->N,N)
        ^
../include/Lcommon.h:51:30: note: in definition of macro 'loop'
 #define loop(i,m,n) for(auto i=(m); i!=(n); i++)
                              ^

I guess the -std=c11 is missing in the gcc invocation...

[fbissey]

Of course I tested with compilers that default to C++11 or C++14 so that would make sense. Will fix ASAP.

[fbissey]

lcalc has both spkg-build and spkg-install, the -std=c++11 was only added to spkg-install which explains the result. I am going to further inspect those for improvement.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​7cf967f

make sure -std=c++11 is passed during build. Various improvement to spkg-build and spkg-install.

[fbissey]

Back to need review.

[dimpase]

over to the bots!

[fbissey]

Replying to dimpase:

over to the bots!

Thank you Dima. Once we have closure here, I would appreciate if you could push the review button on #25532 for the ntl upgrade.