Problem with disabled default construction in use with std.container about gmp-d

[nordlow]

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

[nordlow]

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

[nordlow]

Bug in toString has been fixed on master.

Caused by behaviour of __gmpz_sizeInBase being really crappy.

[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 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;

[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!