Problem with new mpirxx.h

[Jean-Pierre Flori]

It seems that commit a81efe8936cb997d2c0e29fbab0863099898a18d broke the C++ header, at least with recent version of GCC.
It complains about ambiguous overloading of the = operator.

[Brian Gladman]

I don't use GCC so I can't debug this - we need a C++ guru who also uses
C++ to take a look at this.

But I wonder if these lines (around line 1600 in mpirxx.h):

#ifdef MPIRXX_HAVE_LLONG
__gmp_expr & operator=(signed long long int i) { mpz_set_si(mp, i);
return *this; }
__gmp_expr & operator=(unsigned long long int i) { mpz_set_ui(mp, i);
return *this; }
#endif
#if defined( _STDINT_H ) || defined ( _STDINT_H_ ) || defined ( _STDINT )
__gmp_expr & operator=(intmax_t i) { mpz_set_sx(mp, i); return *this; }
__gmp_expr & operator=(uintmax_t i) { mpz_set_ux(mp, i); return *this; }
#endif

It is possible that 'long long' types and 'intmax_t' types are not
distinguishable so it sees two overloads that it cannot disambiguate.

It might be worth commenting out one of these declarations to see if
this fixes the issue. If it does, we then have to think about a solution.

Brian


>

[degski]

"It is possible that 'long long' types and 'intmax_t' types are not
distinguishable so it sees two overloads that it cannot disambiguate."

This is the problem:

typedef long long intmax_t;

typedef unsigned long long uintmax_t;

Is what going on on VS2012 c++11, x64.

signed long long int seems to be the same as long long, the same for the unsigned versions.

The GMP signatures for the first two functions are:

void mpz_set_ui (mpz_t rop, unsigned long int op)

void mpz_set_si (mpz_t rop, signed long int op)

So it comes down to the LP64 vs the LLP64, (long is 32-bit on Win and 64-bit on Lin) I guess some macro's to properly deal with the windows case and the bitness of the build are in order.

intptr_t and uintptr_t actually model the long type quite nicely!

Cheers,

degski

>

--
You received this message because you are subscribed to the Google Groups "mpir-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mpir-devel+...@googlegroups.com.
To post to this group, send email to mpir-...@googlegroups.com.
Visit this group at http://groups.google.com/group/mpir-devel.
For more options, visit https://groups.google.com/groups/opt_out.

--

Sign the petition: https://optin.stopwatching.us/

[Brian Gladman]

On 08/08/2013 11:38, degski wrote:

> "It is possible that 'long long' types and 'intmax_t' types are not
> distinguishable so it sees two overloads that it cannot disambiguate."
>
> This is the problem:
>
> typedef long long intmax_t;
> typedef unsigned long long uintmax_t;
>
> Is what going on on VS2012 c++11, x64.

Yes, these are long long types on Windows x64

> signed long long int seems to be the same as long long, the same for the
> unsigned versions.

I understand that 'signed long long' is the same type as 'long long' but
are you saying that 'unsigned long long' is also the same type as
'long long'? That doesn't seem right to me.

> The GMP signatures for the first two functions are:
>

> void *mpz_set_ui* (mpz_t rop, unsigned long int op)
> void *mpz_set_si* (mpz_t rop, signed long int op)

MPIR defines these as:

void mpz_set_ui(mpz_ptr, mpir_ui);
void mpz_set_si(mpz_ptr, mpir_si);

where mpir_ui/si map to long types everywhere except on Windows x64
where they map to long long types.

> So it comes down to the LP64 vs the LLP64, (long is 32-bit on Win and
> 64-bit on Lin) I guess some macro's to properly deal with the windows case
> and the bitness of the build are in order.

I don't understand this - can you elaborate please.

Brian

[Bill Hart]

On 8 August 2013 12:53, Brian Gladman <b...@gladman.plus.com> wrote:

On 08/08/2013 11:38, degski wrote:

> "It is possible that 'long long' types and 'intmax_t' types are not
> distinguishable so it sees two overloads that it cannot disambiguate."
>
> This is the problem:
>
> typedef long long intmax_t;
> typedef unsigned long long uintmax_t;
>
> Is what going on on VS2012 c++11, x64.

Yes, these are long long types on Windows x64

> signed long long int seems to be the same as long long, the same for the
> unsigned versions.

I understand that 'signed long long' is the same type as 'long long' but
are you saying that 'unsigned long long' is also the same type as
'long long'?  That doesn't seem right to me.

He just means unsigned long long is the same as unsigned long long int, I guess.

Certainly the signed and unsigned versions are different.

 

> The GMP signatures for the first two functions are:
>

> void *mpz_set_ui* (mpz_t rop, unsigned long int op)
> void *mpz_set_si* (mpz_t rop, signed long int op)

MPIR defines these as:

  void mpz_set_ui(mpz_ptr, mpir_ui);
  void mpz_set_si(mpz_ptr, mpir_si);

where mpir_ui/si map to long types everywhere except on Windows x64
where they map to long long types.

> So it comes down to the LP64 vs the LLP64, (long is 32-bit on Win and
> 64-bit on Lin) I guess some macro's to properly deal with the windows case
> and the bitness of the build are in order.

I don't understand this - can you elaborate please.

He's just referring to the fact that on 64 bit linux long is 64 bits, whereas on Windows it's 32 bits. So he's recommending that we fix the problem with some macros which distinguish whether we are on Windows or not, and whether the machine is 32 bits or 64 bits.

I guess we were already doing this, but we got it wrong somewhere. Specifically, you cannot overload a function for intmax_t and long long because they are the same type.

On 64 bit linux it is worse because intmax_t, long and long long are effectively all the same type (not actually sure long and long long are the same on 64 bit linux, haven't checked, but it's feasible, as they are both 64 bits).

Bill.

[Brian Gladman]

On 08/08/2013 13:43, Bill Hart wrote:
> On 8 August 2013 12:53, Brian Gladman <b...@gladman.plus.com> wrote:

[snip]

>> where mpir_ui/si map to long types everywhere except on Windows x64
>> where they map to long long types.
>>
>>> So it comes down to the LP64 vs the LLP64, (long is 32-bit on Win and
>>> 64-bit on Lin) I guess some macro's to properly deal with the windows
>> case
>>> and the bitness of the build are in order.
>>
>> I don't understand this - can you elaborate please.
>
> He's just referring to the fact that on 64 bit linux long is 64 bits,
> whereas on Windows it's 32 bits. So he's recommending that we fix the
> problem with some macros which distinguish whether we are on Windows or
> not, and whether the machine is 32 bits or 64 bits.
>
> I guess we were already doing this, but we got it wrong somewhere.
> Specifically, you cannot overload a function for intmax_t and long long
> because they are the same type.

We provide overloads for both signed and unsigned long and long long
types. At the moment we also provide overloads for uintmax_t and
intmax_t types.

Since this overload ought to fail on both Linux (ambiguity with long)
and Windows x64 (ambiguity with long long) we could simply remove it.

But it doesn't fail on Windows x64 (unless the tests don't detect it) so
it seems that we can add an _MSC_VER based exclude to fix this. But it
would be nice to understand why it fails on GCC but not on Visual Studio
- probably some different algorithm for disambiguation maybe.

Brian

[Bill Hart]

Yeah it certainly seems like it ought to fail for a C++ standard reason on both platforms.

I know C is often nominatively typed, so if types have different names, they are different types. But it is done inconsistently. So this may be an area of ambiguity, even in the standard.

Bill.

   Brian

[degski]

Hi Bill, Brian,

 

Sorry, I was of the radar for a bit. Yes, Bill, you understood correctly what I meant.

 

On 8 August 2013 15:43, Bill Hart <goodwi...@googlemail.com> wrote:

On 8 August 2013 12:53, Brian Gladman <b...@gladman.plus.com> wrote:

On 08/08/2013 11:38, degski wrote:

> "It is possible that 'long long' types and 'intmax_t' types are not
> distinguishable so it sees two overloads that it cannot disambiguate."
>
> This is the problem:
>
> typedef long long intmax_t;
> typedef unsigned long long uintmax_t;
>
> Is what going on on VS2012 c++11, x64.

Yes, these are long long types on Windows x64

> signed long long int seems to be the same as long long, the same for the
> unsigned versions.

I understand that 'signed long long' is the same type as 'long long' but
are you saying that 'unsigned long long' is also the same type as
'long long'?  That doesn't seem right to me.

He just means unsigned long long is the same as unsigned long long int, I guess.

 

Yes, just being a bit terse, sorry. 

Certainly the signed and unsigned versions are different.

 

> The GMP signatures for the first two functions are:
>

> void *mpz_set_ui* (mpz_t rop, unsigned long int op)
> void *mpz_set_si* (mpz_t rop, signed long int op)

MPIR defines these as:

  void mpz_set_ui(mpz_ptr, mpir_ui);
  void mpz_set_si(mpz_ptr, mpir_si);

where mpir_ui/si map to long types everywhere except on Windows x64
where they map to long long types.

> So it comes down to the LP64 vs the LLP64, (long is 32-bit on Win and
> 64-bit on Lin) I guess some macro's to properly deal with the windows case
> and the bitness of the build are in order.

I don't understand this - can you elaborate please.

He's just referring to the fact that on 64 bit linux long is 64 bits, whereas on Windows it's 32 bits. So he's recommending that we fix the problem with some macros which distinguish whether we are on Windows or not, and whether the machine is 32 bits or 64 bits.

 

 

The types I mentioned is what VS2012 defines them as, you can see that by hovering over the  types, f.e. intmax_t, you can than see the definitions, it's as Isaid earlier.

 

 

I guess we were already doing this, but we got it wrong somewhere. Specifically, you cannot overload a function for intmax_t and long long because they are the same type.

 

Yes, that's not allowed in C++. 

 

On 64 bit linux it is worse because intmax_t, long and long long are effectively all the same type (not actually sure long and long long are the same on 64 bit linux, haven't checked, but it's feasible, as they are both 64 bits).

Bill.

 

   Brian

--
You received this message because you are subscribed to the Google Groups "mpir-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mpir-devel+...@googlegroups.com.
To post to this group, send email to mpir-...@googlegroups.com.
Visit this group at http://groups.google.com/group/mpir-devel.
For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "mpir-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mpir-devel+...@googlegroups.com.
To post to this group, send email to mpir-...@googlegroups.com.
Visit this group at http://groups.google.com/group/mpir-devel.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[degski]

Hi Brian,

 

It does fail for me though:

 

Z:\VC\x64\include\mpirxx.h(1596): error : invalid redeclaration of member function "__gmp_expr<mpz_t, mpz_t>::__gmp_expr(__int64)" (declared at line 1590)
      __gmp_expr(intmax_t l) { mpz_init_set_sx(mp, l); }
      ^
 
Z:\VC\x64\include\mpirxx.h(1597): error : invalid redeclaration of member function "__gmp_expr<mpz_t, mpz_t>::__gmp_expr(unsigned __int64)" (declared at line 1591)
      __gmp_expr(uintmax_t l) { mpz_init_set_ux(mp, l); }
      ^

This already happens when just including the header mpirxx.h in a project (VS2012/release/x64).

 

Cheers,

 

degski.

   Brian

--
You received this message because you are subscribed to the Google Groups "mpir-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mpir-devel+...@googlegroups.com.
To post to this group, send email to mpir-...@googlegroups.com.
Visit this group at http://groups.google.com/group/mpir-devel.
For more options, visit https://groups.google.com/groups/opt_out.

[Brian Gladman]

On 14/08/2013 11:06, degski wrote:
> Hi Brian,
>
> It does fail for me though:
>
> Z:\VC\x64\include\mpirxx.h(1596): error : invalid redeclaration of member
> function "__gmp_expr<mpz_t, mpz_t>::__gmp_expr(__int64)" (declared at line
> 1590)
> __gmp_expr(intmax_t l) { mpz_init_set_sx(mp, l); }
> ^
>
> Z:\VC\x64\include\mpirxx.h(1597): error : invalid redeclaration of member
> function "__gmp_expr<mpz_t, mpz_t>::__gmp_expr(unsigned __int64)" (declared
> at line 1591)
> __gmp_expr(uintmax_t l) { mpz_init_set_ux(mp, l); }
> ^
> This already happens when just including the header mpirxx.h in a project
> (VS2012/release/x64).

Hi Degski,

That should be detected by the test t-headers.cc, which is simply:

-------------
#include "mpirxx.h"

int
main (void)
{
return 0;
}
-------------

which works fine for me. Which MPIR version are you using?

Brian

[Brian Gladman]

On 14/08/2013 11:06, degski wrote:

> Hi Brian,
>
> It does fail for me though:
>
> Z:\VC\x64\include\mpirxx.h(1596): error : invalid redeclaration of member
> function "__gmp_expr<mpz_t, mpz_t>::__gmp_expr(__int64)" (declared at line
> 1590)
> __gmp_expr(intmax_t l) { mpz_init_set_sx(mp, l); }
> ^
>
> Z:\VC\x64\include\mpirxx.h(1597): error : invalid redeclaration of member
> function "__gmp_expr<mpz_t, mpz_t>::__gmp_expr(unsigned __int64)" (declared
> at line 1591)
> __gmp_expr(uintmax_t l) { mpz_init_set_ux(mp, l); }
> ^
> This already happens when just including the header mpirxx.h in a project
> (VS2012/release/x64).

Looking at the tests, there is no test that checks things when <cstdint>
is included. I suspect that this is why I don't see these errors.

Brian

[Brian Gladman]

As I suspected, when I include <cstdint> the errors turn up.

So when <cstdint> is included, we need to:

(a) check if (u)intmax_t is a 'long' and, if it is:
(b) check if there is an equivalent 'long' overload;
(c) if there is, don't declare any overloads on (u)intmax_t

(d) check if (u)intmax_t is a 'long long' and, if it is:
(e) check if there is an equivalent 'long long' overload;
(f) if there is, don't declare any overloads on (u)intmax_t

If there are no situations where (u)intmax_t is not either 'long' or
'long long' we could simply remove all these declarations.

Otherwise I wonder what are the right set of MACROS to achieve the above
guards?

Brian

[Brian Gladman]

Answering (partly) my own question, it seems that C++11 has the solution
using <type_traits>:

---------------------------------------------
#include <type_traits>
template< class T, class U > struct is_same;
---------------------------------------------

so we could just remove these defines for 'old' use and add this
solution for 'new' C++ code. But this would mean adding <type_traits>
to mpirxx.h. Would this cause any problems?



>
> Brian
>

[degski]

Hi Brian,

 

I included the header in an existing project, which already had <cstdint> indeed, did your test, and hoppa.

 

Isn't the base of the problem in the fact that the overloads of

 

void mpz_set_ui (mpz_t rop, unsigned long int op)

void mpz_set_si (mpz_t rop, signed long int op)

have signatures with (unsigned) long long int, while signatures of the functions have unsigned long int.

 

degski.

 

   Brian

--
You received this message because you are subscribed to the Google Groups "mpir-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mpir-devel+...@googlegroups.com.
To post to this group, send email to mpir-...@googlegroups.com.
Visit this group at http://groups.google.com/group/mpir-devel.
For more options, visit https://groups.google.com/groups/opt_out.

[degski]

But this would mean adding <type_traits>
to mpirxx.h. Would this cause any problems?

 

I everyone had a c++11 compliant compiler and it's implemented correctly, don't think so, I think some macros would be a simpeler solution.

 

Basically the only problem is 64 bit windows as the 32 bit case is the same as linux.

 

so have

 

__gmp_expr(signed long int l) { mpz_init_set_si(mp, l); }
  __gmp_expr(unsigned long int  l) { mpz_init_set_ui(mp, l); }

#if defined( _WIN64 )
  __gmp_expr(intmax_t l) { mpz_init_set_sx(mp, l); }
  __gmp_expr(uintmax_t l) { mpz_init_set_ux(mp, l); }
#endif

 

and not the (unsigned) long long int ones.

 

Maybe the standard int macro needs to be there, dunno and the version check macro, sure you know better

 

d.

[Brian Gladman]

On 14/08/2013 12:26, degski wrote:
> Hi Brian,
>
> I included the header in an existing project, which already had <cstdint>
> indeed, did your test, and hoppa.
>
> Isn't the base of the problem in the fact that the overloads of
>

> void *mpz_set_ui* (mpz_t rop, unsigned long int op)
> void *mpz_set_si* (mpz_t rop, signed long int op)

>
> have signatures with (unsigned) long long int, while signatures of the

> functions have *unsigned long int.*

Hi Degski,

This is the bit I don't understand. Both mpz_set_ui and mpz_set_si are
C functions defined in mpir.h so the concept of overloading surely
doesn't apply to them?

Brian

[degski]

Hi Brian,

 

No, but for them the long convention works, it's when we start to introduce unsigned long long int as an overload of that function, the conflict arises (on 64 bit windows). I've modified and tested mpirxx.h (2.6.0) on 32 and 64 bit. I'll attach it. It's not possible now though to init/assign an mpz_t with a LL or ULL on 32 bit though, but it looks like GMP never intended that to be possible.

 

cheers,

 

d.

  Brian

--
You received this message because you are subscribed to the Google Groups "mpir-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mpir-devel+...@googlegroups.com.
To post to this group, send email to mpir-...@googlegroups.com.
Visit this group at http://groups.google.com/group/mpir-devel.
For more options, visit https://groups.google.com/groups/opt_out.

[degski]

Hi Brian,

 

I understand  now why you don't get me, but I'm also confused. I assumed the function prototype of mpz_set_si was as the gmp one void mpz_set_si (mpz_t rop, signed long int op), but it's not, it's void mpz_set_si (mpz_t rop, signed long int op) on 32 bit and void mpz_set_si (mpz_t rop, signed long long int op) on 64 bit.

 

Need to take that into account.

 

Cheers,

 

d.

On 14 August 2013 15:31, Brian Gladman <b...@gladman.plus.com> wrote:

  Brian

--
You received this message because you are subscribed to the Google Groups "mpir-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mpir-devel+...@googlegroups.com.
To post to this group, send email to mpir-...@googlegroups.com.
Visit this group at http://groups.google.com/group/mpir-devel.
For more options, visit https://groups.google.com/groups/opt_out.

[degski]

Hi Brian,

It seems to work now, correctly, also possible to construct/assign LL
and ULL to mpz_class on 32 bit.


#include <cstdint>
#include <mpirxx.h>

int main ( ) {

mpz_class test ( 45323453452343ULL );
}

Cheers,

d.

PS: I'll have to go for a while, I'll check back later today...

On 14 August 2013 15:31, Brian Gladman <b...@gladman.plus.com> wrote: