Fix issue 20049 - object.destroy doesn't propagate attributes

[ShigekiKarita]

A declaration of rt_finalize in object.d have not match to its implementation

druntime/src/rt/lifetime.d

Line 1423 in 3ba98e6

extern (C) void rt_finalize(void* p, bool det = true) nothrow

Because of this mismatch, we could not destroy class objects in nothrow.

PS. This issue is found in mir-algorithm libmir/mir-algorithm#208

[dlang-bot]

Thanks for your pull request and interest in making D better, @ShigekiKarita! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✗	20049	major	object.destroy doesn't propagate attributes

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "stable + druntime#2674"

[9il]

Issue https://issues.dlang.org/show_bug.cgi?id=20049

[JinShil]

It appears rt_finalize is indeed nothrow:

druntime/src/rt/lifetime.d

Line 1423 in 8a5e6f2

extern (C) void rt_finalize(void* p, bool det = true) nothrow

[thewilsonator]

If this fixes the linked issue please change the commit title to "Fix issue 20049 - [rest of title]".

Also please target stable.

[JinShil]

Also, please add a test case to help prevent a regression.

[ShigekiKarita]

Thanks. Just a moment. I'll fix that.

[thewilsonator]

You've pulled all of masters commits along with it. You need to rebase your commit on top of stable in addition to retargeting the branch in GitHub.

[ShigekiKarita]

@JinShil I added the regression test. Can you check if it is correctly placed (test/exceptions/src/nothrow_dtor.d)?

[JinShil]

@ShigekiKarita Can you just add a unittest in object.d just below the destroy implementation? I think something like this should work:

// Test to ensure proper behavior of `nothrow` destructors
unittest
{
    class C
    {
        static int dtorCount = 0;
        this() nothrow {}
        ~this() nothrow { dtorCount++; }
    }

    auto c = new C;
    destroy(c);
    assert(C.dtorCount == 1);
}

Edit:
Actually you might need something like this:

unittest
{
    class C
    {
        static int dtorCount = 0;
        this() nothrow {}
        ~this() nothrow { dtorCount++; }
    }

    void test() nothrow
    {
        auto c = new C;
        destroy(c);
        assert(C.dtorCount == 1);
    }
    
    test();
}

[ShigekiKarita]

Thanks. I think nothrow unittest is also fine. This is because current dmd+druntime (2.087.0) cannot compile that.

[thewilsonator]

Please reference the buzilla issue number, otherwise looks good to go.

[ShigekiKarita]

Thanks.