advanpix / mpreal

GNU General Public License v3.0
84 stars 19 forks source link

Fixing up mpreal for modern compilers #17

Closed msoos closed 2 years ago

msoos commented 2 years ago

Fixes compile issue with modern compilers. mpfr_srcptr() shadows mpfr_srcptr which fails the compile on both clang version 13.0.1 and gcc (GCC) 11.2.0.

advanpix commented 2 years ago

This issue was resolved a while ago, see https://github.com/advanpix/mpreal/issues/7. Could you confirm the GCC 11.2.0 still has issues with latest version of MPFR C++? What kind of errors it generates?

msoos commented 2 years ago

Hi,

I would get this weird compile error (this is gcc):

/home/soos/development/sat_solvers/sharpsat-td/src/mpfr/mpreal.h:590:41: fatal error: too many arguments to function call, expected 0, have 1
    mpfr_init2(mpfr_ptr(),mpfr_get_prec(u.mpfr_srcptr()));
                          ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
/usr/include/mpfr.h:866:53: note: expanded from macro 'mpfr_get_prec'
#define mpfr_get_prec(_x) MPFR_VALUE_OF(MPFR_SRCPTR(_x)->_mpfr_prec)
                                        ~~~~~~~~~~~~^~~
/usr/include/mpfr.h:850:65: note: expanded from macro 'MPFR_SRCPTR'
#define MPFR_SRCPTR(x) ((mpfr_srcptr) (0 ? (x) : (mpfr_srcptr) (x)))
                                                  ~~~~~~~~~~~   ^
/usr/include/mpfr.h:845:33: note: expanded from macro 'MPFR_VALUE_OF'
#define MPFR_VALUE_OF(x)  (0 ? (x) : (x))
                                ^
/home/soos/development/sat_solvers/sharpsat-td/src/mpfr/mpreal.h:324:19: note: 'mpfr_srcptr' declared here
    ::mpfr_srcptr mpfr_srcptr() const;
                  ^

But of course too many arguments to function call, expected 0, have 1 makes no sense, it needs 1 argument.

I use Arch Linux, fully updated, latest mpfr library here is 4.1.0p13-2, which is what I have. I'd wager it makes sense to have a get_ function, rather than a function name that conflicts with the struct name? It may also be confusing to the IDEs, as it would be sometimes unclear what you are referencing, the function name or the struct. It may also make it harder to give meaningful error messages (as per above). So I'd argue for a function name that doesn't clash with the struct name, but my experience in software engineering is likely not as expansive as yours, so take my advice with a pinch of salt :)

I hope the above helped,

Mate

advanpix commented 2 years ago

Could you try compiling with MPFR_USE_NO_MACRO defined?

This incompatibility was introduced in recent versions of MPFR, and it can be easily disabled by defining MPFR_USE_NO_MACRObefore including mpfr.h. The mpreal does this automatically and it compiles fine if you just include mpreal.h. I suspect your code includes mpfr.h somewhere in other places without the MPFR_USE_NO_MACRO.

Btw, there is no issue in having a struct and member function of the same name. The issue is that MPFR developers started to use mpfr_srcptr in ambiguous way in MPFR_SRCPTR, so that compiler having difficulty to understand what to use - a struct or a function.

Renaming is not easy, mpreal is ~14 years old, lots of software depends on it.

msoos commented 2 years ago

Hi,

Ah, I see. Yep, add_definitions( -DMPFR_USE_NO_MACRO ) in CMakeLists.txt fixes the issue.

Closing. Sorry, I was just sad to have to deal with this incredibly weird error message that both gcc and clang provided. So confusing. This works, sad to see that there was a mess with macros. Macros often lead to a mess :)

Thanks again,

Mate