Macaulay2 / M2

The primary source code repository for Macaulay2, a system for computing in commutative algebra, algebraic geometry and related fields.
https://macaulay2.com
347 stars 231 forks source link

givaro_bool_constant does not name a type #2347

Closed jkyang92 closed 2 years ago

jkyang92 commented 2 years ago

The following line 20 in aring-zzp-ffpack.hpp is causing compilation issues in aring-zzp-ffpack.cpp for me on Gentoo, due to givaro_bool_constant not being defined

#define bool_constant givaro_bool_constant

Looking at the logs, it seems this is to fix some c++17/14 compatability issues with givaro and ffpack? But since givaro is the one defining the potentially conflicting synonym, it seems to me there's no reason not to just delete this line (and the associated undef). In particular, both givaro and c++17 agree that bool_constant<B> is the same as integral_constant<bool,B>, so I don't see why we need to force ffpack to use givaro_bool_constant. In particular deleting the line works for me.

Also what systems were having this issue to start with, and do they still have this issue? Deleting all references to givaro_bool_constant still lets me compile on both Gentoo and Arch.

d-torrance commented 2 years ago

I'm getting the same error now in Debian unstable:

g++ -pipe -Wall -Wno-shadow -Wcast-qual -Wno-sign-conversion -Wno-sign-compare -Wno-parentheses -Wno-sign-compare  -Wuninitialized -I/usr/include  -Wno-error=pragmas -std=gnu++14 -g -ffile-prefix-map=/build/macaulay2-1.19.1.1+git28.854afd6+ds=. -fstack-protector-strong -Wformat -Werror=format-security -g3 -O2  -Wno-mismatched-tags -Wno-unused-local-typedefs -Wno-unknown-pragmas -std=gnu++17 -Wno-mismatched-tags -I. -DNDEBUG -I/build/macaulay2-1.19.1.1+git28.854afd6+ds/M2/include -I/build/macaulay2-1.19.1.1+git28.854afd6+ds/M2/include -I/build/macaulay2-1.19.1.1+git28.854afd6+ds/M2/usr-host/include -I/build/macaulay2-1.19.1.1+git28.854afd6+ds/M2/include -I/build/macaulay2-1.19.1.1+git28.854afd6+ds/M2/include -I/build/macaulay2-1.19.1.1+git28.854afd6+ds/M2/usr-host/include -I/build/macaulay2-1.19.1.1+git28.854afd6+ds/M2/include -I/build/macaulay2-1.19.1.1+git28.854afd6+ds/M2/include -I/build/macaulay2-1.19.1.1+git28.854afd6+ds/M2/usr-host/include -Wdate-time -D_FORTIFY_SOURCE=2 -isystem /usr/include/libxml2   -DSING_NDEBUG -DOM_NDEBUG -DSING_NDEBUG -DOM_NDEBUG -I/usr/include/x86_64-linux-gnu/singular -I/usr/include/singular -I/usr/include/x86_64-linux-gnu/singular -I/usr/include/singular -I/usr/include/x86_64-linux-gnu/singular -I/usr/include/singular   -I/usr/src/gtest  -Wdate-time -D_FORTIFY_SOURCE=2 -DNDEBUG  -I/usr/include/cddlib -I/usr/include/cdd -I/usr/include/eigen3 -I/usr/include/python3.9 -I/usr/include -DBOOST_STACKTRACE_LINK -isystem /usr/include/libxml2   -DSING_NDEBUG -DOM_NDEBUG -DSING_NDEBUG -DOM_NDEBUG -I/usr/include/x86_64-linux-gnu/singular -I/usr/include/singular -I/usr/include/x86_64-linux-gnu/singular -I/usr/include/singular -I/usr/include/x86_64-linux-gnu/singular -I/usr/include/singular   -I/usr/src/gtest  -Wdate-time -D_FORTIFY_SOURCE=2 -DNDEBUG  -I/usr/include/cddlib -I/usr/include/cdd -I/usr/include/eigen3 -I/usr/include/python3.9 -I/usr/include -DBOOST_STACKTRACE_LINK -isystem /usr/include/libxml2   -DSING_NDEBUG -DOM_NDEBUG -DSING_NDEBUG -DOM_NDEBUG -I/usr/include/x86_64-linux-gnu/singular -I/usr/include/singular -I/usr/include/x86_64-linux-gnu/singular -I/usr/include/singular -I/usr/include/x86_64-linux-gnu/singular -I/usr/include/singular   -I/usr/src/gtest  -Wdate-time -D_FORTIFY_SOURCE=2 -DNDEBUG  -I/usr/include/cddlib -I/usr/include/cdd -I/usr/include/eigen3 -I/usr/include/python3.9 -I/usr/include -DBOOST_STACKTRACE_LINK -I../d -I./../c -I./../d -Wno-fatal-errors -Wno-unused-variable -I./memtailor -I./mathic -I./mathicgb  -c -Wno-cast-qual aring-tower.cpp -o aring-tower.o
In file included from aring-tower.hpp:7,
                 from aring-tower.cpp:3:
aring-zzp-ffpack.hpp:20:23: error: 'givaro_bool_constant' does not name a type; did you mean 'bool_constant'?
   20 | #define bool_constant givaro_bool_constant
      |                       ^~~~~~~~~~~~~~~~~~~~
make[4]: *** [Makefile.common:33: aring-tower.o] Error 1

I just uploaded givaro 4.2.0 to Debian a few days ago, so I'm sure that's related.

mahrud commented 2 years ago

@mikestillman do you still want to remove givaro in favor of using flint more?

jkyang92 commented 2 years ago

So based on the build failures in #2362 it seems we need the define on macs, although I'd have to admit I'm surprised that this behavior is platform dependent. I guess that the "right" solution is for givario to only define bool_constant if the c++ version is <17. But I'm now really confused why I even get the compilation error with the define on linux. In particular, why is it complaining about the #define rather than the point at which the define is used.

jkyang92 commented 2 years ago

I figure out why exactly this error is occurring.

  1. the #include <iostream> line indirectly includes type_traits, which defines std::bool_constant
  2. ffpack includes some Givaro headers
  3. Givaro includes indirectly some header that uses bool_constant (it's <utility> for me), but at this point it tries to use givaro_bool_constant since we've redefined bool_constant to givaro_bool_constant

Also, I found a (closed) Givaro bug referencing the compilation issue on mac that this define is trying to work around, but it seems to me they misidentified the actual fault. https://github.com/linbox-team/givaro/issues/129. I unfortunately don't have a good way to test any of this on mac, so someone who can actually test this might want to bring it up with them.

d-torrance commented 2 years ago

I'm still having this issue in Debian unstable (using givaro 4.2.0, fflas-ffpack 2.5.0, and gcc 11.2.0) after #2362 was merged.

mahrud commented 2 years ago

Try removing -std=gnu++14.

jkyang92 commented 2 years ago

It's not the -std=gnu++14, at least not for me. On gcc 11.2.1 on Gentoo the following file fails

#include <type_traits>
#define bool_constant givaro_bool_constant
#include <fflas-ffpack/ffpack/ffpack.h>
#undef bool_constant
int main(){}

with the command line g++ -std=gnu++17 test.cpp

The proximate cause of this failure is that for the gcc 11.2.1 on gentoo at least, <utility> unless compiled with gnu++11 or lower, uses bool_constant. And fflas-ffpack/ffpack/ffpack.h (indirectly) includes <utility> before givaro/random-integer.h. A hacky fix is the simply include <utility> before #define bool_constant givaro_bool_constant

DanGrayson commented 2 years ago

That sounds like a fine way to fix it. You may also want to include variant and string.

pzinn commented 2 years ago

same problem on Fedora 35... fixed by https://github.com/Macaulay2/M2/pull/2370