Problem with disabled default construction in use with std.container

[bvoq]

So I have the problem that I want to use RedBlackTree!MpZ or simply Array!MpZ with gmd-d. I cannot instantiate Array!MpZ since gmp.z.MpZ is not copyable.

import gmp.z,std.container,std.stdio;
void main() {
    const a = 73.MpZ, b = 3.MpZ;
    writeln((a+b).toString); //to see if everything works fine
    Array!MpZ d = make!(Array!MpZ)(); d.insert(23.MpZ); //saidly does not work.
}

After enabling your enum useCopy = true; // disable copy construction for now on line 132 in z.d I can instantiate this array, however I directly get a malloc error as soon as I try to insert something: moredtesting(88429,0x7ffff19fd3c0) malloc: *** error for object 0x7fc895403a30: pointer being freed was not allocated (Also it seems MpQ doesn't even have the option of enabling useCopy)

Additionally another error seems to come up when instantiating a RedBlackTree: RedBlackTree!MpZ rb = new RedBlackTree!MpZ(); rb.insert(3.MpZ);, since it requires an enabled default construction. Again I can circumvent this by commenting //@disable this(); on line 76 in z.d.

I have the feeling that I'm not supposed to do these changes (I directly get malloc errors and am fairly new to D), so what do you recommend as a work around in order to use MpZ together with std.container? Since it has to do with malloc I thought this might be due to the recent changes done there.

[nordlow]

I can add a template parameter to MpZ that enables copy-construction today. That's gonna solve your problem.

[bvoq]

Thank you so much!

[nordlow]

You can, on master, try using CopyableMpZ instead of MpZ and see if things work.

[bvoq]

Yes, this worked quite well. I still cannot instantiate a RedBlackTree without enabling the default constructor: //@disable this(); Can this do any harm?

[nordlow]

In D, you cannot change the behaviour of default constructors, which do a full shallow zero-initialization of the struct.

This is not compatible with how GNU MP's mpz_init fills the contents of the C struct __mpz_struct.

To enable default construction for MpZ, will require all the functions that operator on MpZ-instance(s) to check if they have been initialized; if they haven't they must call initialize() that in turn calls mpz_init(). I'm gonna ask on the forums if there is a pragmatic solution to this problem.

For details on the problem see: https://wiki.dlang.org/Language_issues#Default_constructors

I'm looking into it right now.

[nordlow]

I've pushed a load of fixes and tests for supporting default-construction of MpZ. Should work now.

[nordlow]

I forgot to tell you that you can use

import std.typecons : RefCounted;
alias RcMpZ = RefCounted!MpZ;

to get reference counting (RC) semantics for the non-copyable MpZ. This in order to avoid inadvertent calls to the copy constructor of MpZ. That makes you loose pure-ness of destruction because the destructor of RefCounted hasn't yet been pureified because no pureFree has yet been added. I'm working on that now.

[bvoq]

Thanks! The changes you made solved the problem. Using RefCounted makes sense, but now RBTrees work even without this feature! The only thing I came across was that some MpZ values have the terminating symbol () attached to it when running toString. I just hacked something in to remove this character if it appeared. The only numbers affected seem to lie between a power of 2 and the the power of 10.

for(int i = -100; i < 10000; ++i) {
    writeln(i.Z); //for example all characters between 8192-9999 have this terminating symbol attached to it.
}

Quickfix:

string actualString = str[0] == '-' ? str : str.ptr[0 .. size];
string removedWeirdAscii = !(actualString[$-1] >= '0' && actualString[$-1] <= '9') ? actualString[0 .. $-1] : actualString;
return removedWeirdAscii;

[nordlow]

Bug in toString has been fixed on master.

Caused by behaviour of __gmpz_sizeInBase being really crappy.

[bvoq]

Ok cool! I've came across just one more thing. So I'm using your default constructed CopyableMpZ, but I get the following error, whenever the garbage collector decides to deallocate the object. mydprogram(34146,0x7fffc7a083c0) malloc: *** error for object 0x7febc17336b0: pointer being freed was not allocated *** set a breakpoint in malloc_error_break to debug

I've fixed this by adding a default initialised bool hasBeenInit = false; Then on every occurence of __gmpz_init(_ptr); version(ccc) { ++_ccc; } I set the boolean to true.

And lastly I had to patch the destructor:

    /// Destruct `this`.
    ~this() @trusted
    {
        assert(_ptr, "Pointer is null");
        if(hasBeenInit) {
            __gmpz_clear(_ptr); version(ccc) { ++_ccc; }
        }
    }

This fix leads to assertion errors in q.d when running the unittests though :| So for now I've simply disabled this line in the destructor: __gmpz_clear(_ptr); version(ccc) { ++_ccc; }

[nordlow]

I'll look into it this weekend.

[bvoq]

Cool. Thanks!

[nordlow]

Can you try this fix on master?

Note that removing the call to __gmpz_clear is gonna cause massive memory leaks in your application.

If the memory problem still remains, can you please add a test case in this thread that triggers the error?

[bvoq]

Hmm.. I still run into the same error. I can try to isolate the issue tomorrow.

[nordlow]

Ok, thanks.

[nordlow]

Any news on isolating the bug?

[bvoq]

No, I'll look into it more, sorry for taking so much time. For some reason wrapping it in if statements works, but I didn't find the cause of the problem (I begin to think it might just be on my side, so you might want to close it for now.)

[bvoq]

The code works well, the problem was on my side! Cheers

[nordlow]

Great!