Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] 0.24 fails with mpfr-4.1.0: error: expected unqualified-id #281

Closed
yurivict opened this issue Aug 7, 2021 · 13 comments
Closed

[BUG] 0.24 fails with mpfr-4.1.0: error: expected unqualified-id #281

yurivict opened this issue Aug 7, 2021 · 13 comments
Labels

Comments

@yurivict
Copy link

yurivict commented Aug 7, 2021

Describe the bug
This started when math/mpfr port was updated:

/wrkdirs/usr/ports/math/mppp/work/mppp-0.24/src/complex.cpp:662:7: error: expected unqualified-id
    ::mpfr_set(mpc_realref(&m_mpc), tmp.get_mpfr_t(), MPFR_RNDN);
      ^
/usr/local/include/mpfr.h:882:3: note: expanded from macro 'mpfr_set'
  __extension__ ({                              \
  ^

Log: http://beefy16.nyi.freebsd.org/data/130amd64-default/1684f47e9bea/logs/mppp-0.24_2.log (IPv6 URL)

OS: FreeBSD 13

@yurivict yurivict added the bug label Aug 7, 2021
@bluescarni
Copy link
Owner

@yurivict This is quite weird... It turns out there's a switch in MPFR to select whether certain functions are implemented as macros rather than "real" C functions. The compilation error happens because mp++ is calling mpfr functions as if they were C functions (i.e., ::mpfr_set_...()) when instead they are implemented as macros.

Strangely enough, I never ran into this issue. Can you check your mpfr.h and tell me if the MPFR_USE_NO_MACRO name is defined anywhere?

@yurivict
Copy link
Author

yurivict commented Aug 7, 2021

MPFR_USE_NO_MACRO isn't defined in mpfr.h. There is only #ifndef MPFR_USE_NO_MACRO with it.

@bluescarni
Copy link
Owner

@yurivict I checked again in my mpfr.h header and I noticed that the function version of mpfr_set() is there as well. For me it is around line 500:

__MPFR_DECLSPEC int mpfr_set4 (mpfr_ptr, mpfr_srcptr, mpfr_rnd_t, int);
__MPFR_DECLSPEC int mpfr_abs (mpfr_ptr, mpfr_srcptr, mpfr_rnd_t);
__MPFR_DECLSPEC int mpfr_set (mpfr_ptr, mpfr_srcptr, mpfr_rnd_t); // <-------
__MPFR_DECLSPEC int mpfr_neg (mpfr_ptr, mpfr_srcptr, mpfr_rnd_t);
__MPFR_DECLSPEC int mpfr_signbit (mpfr_srcptr);

Do you also have these declarations in your mpfr.h?

@yurivict
Copy link
Author

yurivict commented Aug 7, 2021

Do you also have these declarations in your mpfr.h?

Yes.

@bluescarni
Copy link
Owner

Do you also have these declarations in your mpfr.h?

Yes.

And they are not deactivated by any #ifdef or similar?

@yurivict
Copy link
Author

yurivict commented Aug 7, 2021

Here is this file: https://people.freebsd.org/~yuri/mpfr.h

@bluescarni
Copy link
Owner

I can't really figure out what is going on. My mpfr.h does not seem to differ from yours, yet all my compilers here do not complain (including clang).

Running g++ -E on complex.cpp and examining the output on the offending line that generates the error on FreeBSD, yields this:

complex &complex::abs()
{
    static thread_local real tmp;

    tmp.set_prec(get_prec());
    ::mpc_abs(tmp._get_mpfr_t(), &m_mpc, MPFR_RNDN);

    ::
# 679 "complex.cpp" 3 4
     mpfr_set4(((
# 679 "complex.cpp"
     &m_mpc
# 679 "complex.cpp" 3 4
     )->re),
# 679 "complex.cpp"
     tmp.get_mpfr_t()
# 679 "complex.cpp" 3 4
     ,
# 679 "complex.cpp"
     MPFR_RNDN
# 679 "complex.cpp" 3 4
     ,((
# 679 "complex.cpp"
     tmp.get_mpfr_t()
# 679 "complex.cpp" 3 4
     )->_mpfr_sign))
# 679 "complex.cpp"
                                                               ;

So it seems to me like the compiler is indeed expanding the mpfr_set() call via the macro, and the leading :: is ignored. Similarly, clang++ -E outputs:

complex &complex::abs()
{
    static thread_local real tmp;

    tmp.set_prec(get_prec());
    ::mpc_abs(tmp._get_mpfr_t(), &m_mpc, MPFR_RNDN);

    ::mpfr_set4(((&m_mpc)->re),tmp.get_mpfr_t(),MPFR_RNDN,((tmp.get_mpfr_t())->_mpfr_sign));
    ::mpfr_set_zero(((&m_mpc)->im), 1);

    return *this;
}

So mpfr_set() is expanded to ::mpfr_set4... here too.

Would you be able to see what clang++ -E outputs in your build?

@yurivict
Copy link
Author

yurivict commented Aug 8, 2021

This command:

/usr/bin/c++  -Dmppp_EXPORTS -I/disk-samsung/freebsd-ports/math/mppp/work/mppp-0.24/include -Iinclude -isystem /usr/local/include -O2 -pipe -fno-omit-frame-pointer -fstack-protector-strong -fno-strict-aliasing -fno-omit-frame-pointer -O2 -pipe -fno-omit-frame-pointer -fstack-protector-strong -fno-strict-aliasing -fno-omit-frame-pointer -flto=thin -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -ftemplate-depth=1024 -fdiagnostics-show-template-tree -MD -MT CMakeFiles/mp++.dir/src/complex.cpp.o -MF CMakeFiles/mp++.dir/src/complex.cpp.o.d -E /disk-samsung/freebsd-ports/math/mppp/work/mppp-0.24/src/complex.cpp -o complex.cpp.txt

outputs this: https://people.freebsd.org/~yuri/complex.cpp.txt

@bluescarni
Copy link
Owner

@yurivict thanks for checking! The relevant part of the file is this:

complex &complex::abs()
{
    static thread_local real tmp;

    tmp.set_prec(get_prec());
    ::mpc_abs(tmp._get_mpfr_t(), &m_mpc, MPFR_RNDN);

    ::__extension__ ({ mpfr_srcptr _p = (tmp.get_mpfr_t()); mpfr_set4(((&m_mpc)->re),_p,MPFR_RNDN,((_p)->_mpfr_sign)); });
    ::mpfr_set_zero(((&m_mpc)->im), 1);

    return *this;
}

So, for some reason, the mpfr_set() macro expands to something else in my setups (Linux/OSX/Windows). I cannot really understand how this happens...

However, it seems pretty clear at this point that I need to remove all the leading :: in the mp++ source code when invoking MPFR macros. I'll be on vacation the next couple of weeks, but as soon as I get back I'll open a PR and, after we can ensure that all such occurrences are caught and fixed, will make a new mp++ release.

Thanks a lot for reporting, I'll ping back here when I start fixing mp++.

@yurivict
Copy link
Author

yurivict commented Aug 8, 2021

Thanks!

Have a happy vacation!

Yuri

@bluescarni
Copy link
Owner

Thanks!

@bluescarni
Copy link
Owner

@yurivict There's a PR up at #282 that should fix this issue. Could you check if the PR does indeed fix the problem?

@yurivict
Copy link
Author

The problem is now gone, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants