This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Add deduction guides for shared_ptr and weak_ptr
ClosedPublic

Authored by logan-5 on Oct 29 2019, 9:58 PM.

Details

Summary

This patch adds deduction guides to <memory> to allow deducing construction of shared_ptrs from unique_ptrs, and from weak_ptrs and vice versa, as specified by C++17.

Diff Detail

Repository
rCXX libc++

Event Timeline

logan-5 created this revision.Oct 29 2019, 9:58 PM
logan-5 updated this revision to Diff 227139.Oct 30 2019, 10:31 AM

Add a newline to synopsis

The tests should be more comprehensive; they should ASSERT_SAME_TYPE(decltype(s), XXX) to make sure that the deduction guides actually return the correct type, rather than just compile.

libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/deduction.pass.cpp
25

When you're testing type-deduction, I find it much easier to convince myself that the types are correct if you are explicit whenever possible (instead of just auto everywhere)
Example:

using SP = std::shared_ptr<A>;
using WP = std::weak_ptr<A>;

SP s0 = new A;
WP w = s0;
auto s = std::shared_ptr(w);
ASSERT_SAME_TYPE(decltype(s), SP);

Also, you should check the value of s. Make sure it points to the same thing as s0.

mclow.lists requested changes to this revision.Oct 31 2019, 11:34 PM
This revision now requires changes to proceed.Oct 31 2019, 11:34 PM
logan-5 updated this revision to Diff 227493.Nov 1 2019, 11:39 AM

Beefed up the tests in response to some feedback -- thanks.

logan-5 marked an inline comment as done.Nov 18 2019, 11:12 AM

ping

Hello! Pinging this again.

mclow.lists accepted this revision.Dec 29 2019, 8:44 PM

This looks good to me now.
You need to update www/cxx17_status.html appropriately as well.

This revision is now accepted and ready to land.Dec 29 2019, 8:44 PM

Thanks Marshall. As for www/cxx1z_status.html, my best guess is that it doesn't need updating, since the row for P0433R2 lists it as In progress, which I believe is still the case after this patch. Please correct me if I'm wrong. Otherwise, I'd be grateful if someone with commit access could help me tie a bow around this.

Giving this another ping--I just need someone to help me commit this.

Thanks! Committed as

commit 83564056d4b186c9fcf016cdbb388755009f7b5a
Author: Logan Smith <logan.r.smith0@gmail.com>
Date:   Thu May 7 12:07:01 2020 -0400

    [libcxx] Add deduction guides for shared_ptr and weak_ptr

    This patch adds deduction guides to <memory> to allow deducing
    construction of shared_ptrs from unique_ptrs, and from weak_ptrs
    and vice versa, as specified by C++17.

    Differential Revision: https://reviews.llvm.org/D69603
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 7 2020, 9:40 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
logan-5 marked an inline comment as done.May 8 2020, 10:58 AM

Ack. It fails because there's a leak because a dummy deleter I used (D in deduction.pass.cpp) is a no-op. I suppose it should delete its argument as expected.

Should I submit another patch? Or does someone have the power to just commit the fix directly?

libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/deduction.pass.cpp
27

Here's the offending line.