Opened 3 years ago

Closed 3 years ago

#24029 closed enhancement (fixed)

Force -std=gnu++11 for Linbox

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.1
Component: packages: standard Keywords:
Cc: fbissey, cpernet Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 39e0a08 (Commits) Commit: 39e0a08b7a1c73fdb34478fb47fecfcfd53dcb85
Dependencies: Stopgaps:

Description

It turns out that Linbox compiles with -std=gnu++11 but not with -std=c++11:

gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -fPIC -I/home/buildbot/slave/sage_git/build/local/include -I/home/buildbot/slave/sage_git/build/local/include/python2.7 -I/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/numpy/core/include -I/home/buildbot/slave/sage_git/build/src -I/home/buildbot/slave/sage_git/build/src/sage/ext -Ibuild/cythonized -I/home/buildbot/slave/sage_git/build/local/include/python2.7 -c build/cythonized/sage/libs/linbox/linbox_flint_interface.cpp -o build/temp.linux-x86_64-2.7/build/cythonized/sage/libs/linbox/linbox_flint_interface.o -fno-strict-aliasing -DCYTHON_CLINE_IN_TRACEBACK=1 -U_LB_DEBUG -DDISABLE_COMMENTATOR -Wall -DNDEBUG -UFFLASFFPACK_DEBUG -D__FFLASFFPACK_HAVE_CBLAS -g -O2 -DFFLAS_COMPILED -DFFPACK_COMPILED -fPIC -mmmx -mpopcnt -msse -msse2 -msse3 -msse4.1 -msse4.2 -mavx -mavx2 -mfma -mbmi -mbmi2 -mfpmath=sse -fabi-version=6 -I/home/buildbot/slave/sage_git/build/local/include/linbox -I/home/buildbot/slave/sage_git/build/local/include -std=c++11
In file included from /usr/include/c++/6/bits/move.h:57:0,
                 from /usr/include/c++/6/bits/nested_exception.h:40,
                 from /usr/include/c++/6/exception:173,
                 from /usr/include/c++/6/ios:39,
                 from /usr/include/c++/6/ostream:38,
                 from /usr/include/c++/6/iostream:39,
                 from /home/buildbot/slave/sage_git/build/local/include/linbox/linbox-config.h:45,
                 from build/cythonized/sage/libs/linbox/linbox_flint_interface.cpp:655:
/usr/include/c++/6/type_traits: In instantiation of ‘struct std::make_unsigned<__int128 unsigned>’:
/home/buildbot/slave/sage_git/build/local/include/givaro/modular-int64.h:46:60:   required from ‘class Givaro::Modular<long int, __int128 unsigned>’
/home/buildbot/slave/sage_git/build/local/include/givaro/modular-int64.inl:32:36:   required from here
/usr/include/c++/6/type_traits:1831:62: error: invalid use of incomplete type ‘class std::__make_unsigned_selector<__int128 unsigned, false, false>’
     { typedef typename __make_unsigned_selector<_Tp>::__type type; };
                                                              ^~~~
/usr/include/c++/6/type_traits:1788:11: note: declaration of ‘class std::__make_unsigned_selector<__int128 unsigned, false, false>’
     class __make_unsigned_selector;
           ^~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/6/type_traits: In instantiation of ‘struct std::make_unsigned<__int128>’:
/home/buildbot/slave/sage_git/build/local/include/givaro/modular-int64.h:46:60:   required from ‘class Givaro::Modular<long int, __int128>’
/home/buildbot/slave/sage_git/build/local/include/givaro/modular-int64.inl:36:35:   required from here
/usr/include/c++/6/type_traits:1831:62: error: invalid use of incomplete type ‘class std::__make_unsigned_selector<__int128, false, false>’
     { typedef typename __make_unsigned_selector<_Tp>::__type type; };
                                                              ^~~~

To make this fact explicit and to fix #23919, I propose to add -std=gnu++11 to the CXXFLAGS of Linbox.

Change History (11)

comment:1 Changed 3 years ago by jdemeyer

  • Cc cpernet added

comment:2 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/force__std_gnu__11_for_linbox

comment:3 Changed 3 years ago by jdemeyer

  • Commit set to 39e0a08b7a1c73fdb34478fb47fecfcfd53dcb85
  • Status changed from new to needs_review

New commits:

39e0a08Force -std=gnu++11 for Linbox

comment:4 Changed 3 years ago by jdemeyer

  • Cc fbissey added

comment:5 Changed 3 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

LGTM

comment:6 Changed 3 years ago by fbissey

Actually I am surprised you didn't bump the version number of linbox. That's out of character for you :)

comment:7 Changed 3 years ago by cpernet

For the record: when compiling LinBox? (and fflas-ffpack and givaro) with clang-3.8 and using the glibc++ v>=6 (IIRC), we hit a bug when the --std=gnu++11 as the compiler does not support the __float128 type. See https://github.com/linbox-team/givaro/issues/47

Replacing std=gnu++11 by std=c++11 in this case solves the problem. Hence, we let givaro/fflas/linbox configure scripts select which option to use and properly export it in the pc files (in upstream version of the lib).

I think this ticket is still good to go for the current version of linbox shipped in Sage, and assuming that one compiles with gcc, but it will probably be removed when the new versions will be ported to sage (which I am working on now). I am just a little worried about having the main sage configure script forcing the gnu++11 option unconditionally.

comment:8 follow-up: Changed 3 years ago by fbissey

We don't force anything. Actually we stopped spkg-install from forcing -std=c++98/gnu++98, it now default to whatever configure default to. We only force gnu++11 in the sage library. I guess I'll see what happens with clang in #12426.

comment:9 in reply to: ↑ 8 Changed 3 years ago by cpernet

Replying to fbissey:

We only force gnu++11 in the sage library.

But since linbox/fflas are source libraries, they are compiled bits by bits while compiling the sage library, and it would therefore require to use the appropriate c++11 flag that linbox/fflas will expose in their pc files.

comment:10 Changed 3 years ago by fbissey

I cannot argue with that. Real consistency will hopefully be achieved in the near future.

comment:11 Changed 3 years ago by vbraun

  • Branch changed from u/jdemeyer/force__std_gnu__11_for_linbox to 39e0a08b7a1c73fdb34478fb47fecfcfd53dcb85
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.