From b6053470714c121731fbc051aea21de4764fd74d Mon Sep 17 00:00:00 2001 From: mohabsafey Date: Sat, 5 Sep 2026 22:18:49 +0200 Subject: [PATCH] fixes include-order fragility across the msolve/libmsolve translation unit Several .c files are textually #included into one translation unit (via msolve.c and libmsolve.c) and silently relied on being processed in a specific order to see functions/macros/types defined by files included later or listed elsewhere. Reordering those #include lines (e.g. an editor auto-formatter alphabetizing them) broke the build with implicit-declaration/conflicting-type errors. Forward-declare the cross-file functions involved (msolve.c's duplicate.c/linear.c/lifting.c/lifting-gb.c block; libmsolve.c's display_monomial_full and next_prime), and have iofiles.c, hilbert.c, msolve-data.c and mpq_reconstruct.c include msolve-data.h themselves instead of relying on an earlier file to have pulled it in. Also fixes crt/mpz_CRT_ui.c's vendored ulong_extras.h, which only avoided its own broken "ulong_extras/ll_mod_preinv.c" include by accident: it shares FLINT's ULONG_EXTRAS_H include guard, so it only worked as long as hilbert.c (via fglm) happened to pull in the real flint/ulong_extras.h first. Now includes the real header directly so the guard is always pre-empted regardless of #include order elsewhere. Verified by fully alphabetizing both include blocks and rebuilding; all 65 tests still pass with the real, unmodified source. Co-Authored-By: Claude Sonnet 5 --- src/crt/mpq_reconstruct.c | 1 + src/crt/mpz_CRT_ui.c | 7 +++++++ src/msolve/hilbert.c | 1 + src/msolve/iofiles.c | 1 + src/msolve/libmsolve.c | 12 ++++++++++++ src/msolve/msolve-data.c | 2 ++ src/msolve/msolve.c | 30 ++++++++++++++++++++++++++++++ 7 files changed, 54 insertions(+) diff --git a/src/crt/mpq_reconstruct.c b/src/crt/mpq_reconstruct.c index 1c9f71d6..f0150cc4 100644 --- a/src/crt/mpq_reconstruct.c +++ b/src/crt/mpq_reconstruct.c @@ -25,6 +25,7 @@ **/ #include +#include "../msolve/msolve-data.h" #include "../msolve/streams.h" /* #define ROT(u,v,t) \ */ diff --git a/src/crt/mpz_CRT_ui.c b/src/crt/mpz_CRT_ui.c index ce24b570..71023d07 100644 --- a/src/crt/mpz_CRT_ui.c +++ b/src/crt/mpz_CRT_ui.c @@ -24,6 +24,13 @@ This implementation is a (very) slight modification of the functions in FLINT. **/ +/* Force the real FLINT ulong_extras.h to be processed first: it and our + * local ulong_extras.h below share the ULONG_EXTRAS_H include guard, so + * whichever is seen first "wins". Without this, our copy only works by + * accident of #include order elsewhere pulling in FLINT's real header + * first; if it doesn't, our copy's own "#include + * ulong_extras/ll_mod_preinv.c" fails since that file isn't vendored here. */ +#include #include "ulong_extras.h" #include "../msolve/streams.h" diff --git a/src/msolve/hilbert.c b/src/msolve/hilbert.c index 41d9699f..d77669df 100644 --- a/src/msolve/hilbert.c +++ b/src/msolve/hilbert.c @@ -21,6 +21,7 @@ #include "../fglm/data_fglm.c" #include "../fglm/libfglm.h" #include "../neogb/meta_data.h" +#include "msolve-data.h" #include "streams.h" #define REDUCTION_ALLINONE 1 diff --git a/src/msolve/iofiles.c b/src/msolve/iofiles.c index c3508ad3..a9239fd2 100644 --- a/src/msolve/iofiles.c +++ b/src/msolve/iofiles.c @@ -20,6 +20,7 @@ #include "getdelim.h" +#include "msolve-data.h" #include "streams.h" static inline void store_exponent(const char *term, data_gens_ff_t *gens, int64_t pos) diff --git a/src/msolve/libmsolve.c b/src/msolve/libmsolve.c index 81cc8739..097940b9 100644 --- a/src/msolve/libmsolve.c +++ b/src/msolve/libmsolve.c @@ -25,6 +25,18 @@ #include "msolve-data.h" #include "msolve-data.c" #include "streams.h" + +/* iofiles.c, hilbert.c, primes.c and msolve.c are included below as a single + * translation unit and some of them call into functions defined further + * down that list (hilbert.c uses iofiles.c's display_monomial_full; + * msolve.c/lifting-gb.c, included via msolve.c, use primes.c's + * next_prime). Forward-declaring them here makes the #include order below + * cosmetic instead of load-bearing. */ +static inline int32_t display_monomial_full(FILE *file, const int nv, + char **vnames, + int64_t pos, int32_t *bexp); +uint32_t next_prime(uint32_t n); + #include "iofiles.c" #include "hilbert.c" #include "primes.c" diff --git a/src/msolve/msolve-data.c b/src/msolve/msolve-data.c index 34e374d2..aaaf23aa 100644 --- a/src/msolve/msolve-data.c +++ b/src/msolve/msolve-data.c @@ -18,6 +18,8 @@ * Christian Eder * Mohab Safey El Din */ +#include "msolve-data.h" + static void initialize_mstrace(mstrace_t msd, md_t *st, bs_t *bs){ msd->lp = (primes_t *)calloc(st->nthrds, sizeof(primes_t)); diff --git a/src/msolve/msolve.c b/src/msolve/msolve.c index 159cc0af..6881f6e9 100644 --- a/src/msolve/msolve.c +++ b/src/msolve/msolve.c @@ -20,6 +20,36 @@ #include "msolve.h" #include "streams.h" + +/* duplicate.c, linear.c, lifting.c and lifting-gb.c are included below as a + * single translation unit and some of them call into functions defined + * further down that list (lifting.c uses linear.c's *_linear_forms helpers, + * lifting-gb.c uses lifting.c's *_rrec_data and duplicate.c's + * duplicate_data_mthread_gbtrace). Forward-declaring them here makes the + * #include order below cosmetic instead of load-bearing, so a formatter (or + * anyone) resorting those lines can no longer break the build. */ +static inline void duplicate_data_mthread_gbtrace(int nthreads, + bs_t *bs, + md_t *st, + int32_t *num_gb, + int32_t **leadmons_ori, + int32_t **leadmons_current, + trace_t **btrace); + +static inline mpz_t *allocate_crt_linear_forms(int nlins, int nv, + uint32_t **lineqs_ptr); +static inline mpz_t *allocate_mpq_linear_forms(int nlins, int nv); +static inline void crt_linear_forms_clear(mpz_t *crt_linear_forms, int nlins, + int nv); +static inline void mpq_linear_forms_clear(mpz_t *mpq_linear_forms, int nlins, + int nv); +static inline mpz_t *mpz_linear_forms_allocate(int nlins, int nv); +static inline void mpz_linear_forms_clear(mpz_t *mpz_linear_forms, int nlins, + int nv); + +void initialize_rrec_data(rrec_data_t recdata); +void free_rrec_data(rrec_data_t recdata); + #include "duplicate.c" #include "linear.c" #include "lifting.c"