Hi all,
I found a bug in Phobos typecons unittest:
https://github.com/ldc-developers/phobos/blob/6c83b490f7d6c66bf430e5249dae608848d3ac2c/std/typecons.d#LL7088C1-L7108C46

pure @system unittest
{
    foreach (MyRefCounted; AliasSeq!(SafeRefCounted, RefCounted))
    {
        MyRefCounted!int* p;
        {
            auto rc1 = MyRefCounted!int(5);
            p = &rc1; // assigns reference to variable in inner scope...
            assert(rc1 == 5);
            assert(rc1._refCounted._store._count == 1);
            auto rc2 = rc1;
            assert(rc1._refCounted._store._count == 2);
            // Reference semantics
            rc2 = 42;
            assert(rc1 == 42);
            rc2 = rc2;
            assert(rc2._refCounted._store._count == 2);
            rc1 = rc2;
            assert(rc1._refCounted._store._count == 2);
        }
        assert(p._refCounted._store == null);  // use after scope!

The bug is uncovered when optimization and variable lifetime are considered with LDC.
I see that the test is trying to prove that the MyRefCounted dtors are run correctly when the scope ends. However, the way that this is tested is technically UB as far as I understand the D lang spec (although I cannot find explicit mention of variable lifetimes, please help...).
I can simply disable optimization for this unittest function (ldc.attributes.optStrategy), but would rather not.

Ideas?

thanks,
Johan

On Monday, 15 May 2023 at 18:55:09 UTC, Johan wrote:

pure @system unittest
{
    foreach (MyRefCounted; AliasSeq!(SafeRefCounted, RefCounted))
    {
        MyRefCounted!int* p;
        {
            auto rc1 = MyRefCounted!int(5);
            p = &rc1; // assigns reference to variable in inner scope...
            assert(rc1 == 5);
            assert(rc1._refCounted._store._count == 1);
            auto rc2 = rc1;
            assert(rc1._refCounted._store._count == 2);
            // Reference semantics
            rc2 = 42;
            assert(rc1 == 42);
            rc2 = rc2;
            assert(rc2._refCounted._store._count == 2);
            rc1 = rc2;
            assert(rc1._refCounted._store._count == 2);
        }
        assert(p._refCounted._store == null);  // use after scope!

I'd simply suggest to remove the assertion or to put it in the inner scope given that the problem is presumably that all allocas for the unittest are done on function entry

On Monday, 15 May 2023 at 18:55:09 UTC, Johan wrote:

That the destructor is run at the end of the scope is the job of the compiler, not something that this unittest needs to test. To test that the destructor nullifies the store, __dtor can be called manually.

On Monday, 15 May 2023 at 20:22:15 UTC, Dennis wrote:

https://github.com/dlang/phobos/pull/8751