flintlib / arb

Arb has been merged into FLINT -- use https://github.com/flintlib/flint/ instead
http://arblib.org/
GNU Lesser General Public License v2.1
456 stars 137 forks source link

fix compilation with musl libc #376

Closed tornaria closed 2 years ago

tornaria commented 3 years ago

In order to use cpu_set_t it is necessary to #define _GNU_SOURCE before including sched.h (in flint).

However, in musl libc sched.h will be included from pthread.h. This means by the time flint headers get to include sched.h, it has already been included from pthread.h so it won't work.

The fix is easy: include flint headers before #include <pthread.h>. This is what this PR does.

This together with wbhart/flint2#988 fixes the issue.

I take the oportunity to also note: a different workaround would be to use -D_GNU_SOURCE as a compile option. This actually causes a different issue: it turns out defining _GNU_SOURCE before including math.h makes the latter define a function fdiv() which then causes a conflict with a static function used in acb_elliptic/rj.c. It might be wise to choose a different name for that function.

rj.c:50:25: error: conflicting types for 'fdiv'
   50 | static __inline__ slong fdiv(slong x, slong y)
      |                         ^~~~
In file included from /home/tornaria/src/flint/arb/arf.h:25,
                 from /home/tornaria/src/flint/arb/acb.h:22,
                 from /home/tornaria/src/flint/arb/acb_elliptic.h:16,
                 from rj.c:12:
/usr/include/bits/mathcalls-narrow.h:27:20: note: previous declaration of 'fdiv' was here
   27 | __MATHCALL_NARROW (__MATHCALL_NAME (div), __MATHCALL_REDIR_NAME (div), 2);
      |                    ^~~~~~~~~~~~~~~
ericonr commented 3 years ago

I take the oportunity to also note: a different workaround would be to use -D_GNU_SOURCE as a compile option

This is, IMO, the preferred option. It'd even be okay to set this in packaging scripts and for this project to not care about it at all.

The only issue is that you'd need to somehow solve the fdiv conflict. Is the function exported to external users? If it's only internal, changing the name should hopefully be simple to do :)

Otherwise, we can explore better ways to avoid the conflict, though I should note that sharing a name with a function provided by libc (or well, libm in this case) can lead to weird issues in some edge cases.

ericonr commented 3 years ago

It says it's a static function, imo the best thing would be renaming it, then.

ericonr commented 2 years ago

Thanks!