[PATCH] D30837: [libcxx] Support for shared_ptr<T()>

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

I notice that the Standard implies that std::unique_ptr<T> will work only with *object types* T, and yet the "object" wording is missing from shared_ptr.

> A unique pointer is an object that owns another *object* and manages that other *object* through a pointer.



> The shared_ptr class template stores a *pointer*, usually obtained via new. shared_ptr implements semantics of shared ownership; the last remaining owner of the pointer is responsible for destroying the object, *or otherwise releasing the resources associated with the stored pointer.*

So it does seem like shared_ptr<T()> should work and should store a T(*)().
And it does seem that unique_ptr<T(), custom_deleter> should continue to not-work... but maybe there's a useful vendor extension or standards proposal to be made in that area.



================
Comment at: include/memory:4209
 {
-    typedef __shared_ptr_emplace<_Tp, allocator<_Tp> > _CntrlBlk;
+    typedef __shared_ptr_emplace<_Tp, allocator<int> > _CntrlBlk;
     typedef allocator<_CntrlBlk> _Alloc2;
----------------
IIUC, you don't need to touch make_shared. I doubt these changes are truly harmful, but I don't understand what meaning we're trying to assign to things like

    auto a = std::make_shared<void()>();
    auto b = std::make_shared<void()>(myFunction);

It seems like the effect of this change is just to mess with anyone who's explicitly specializing std::allocator<MyObjectType> to make its ::rebind member do something wacky. In which case I'd say they've got it coming to them... but the effect of this change doesn't seem relevant to the purpose described in your commit message. :)


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp:49
+struct Result {};
+void resultDeletor(Result (*)()) {}
+Result theFunction() { return Result(); }
----------------
Could you plausibly make this

    void resultDeleter(Result (*f)()) { assert(f == theFunction); }

for extra sanity checking?
For super-duper checking, touch a global variable in `resultDeleter` so that you can verify that the deleter was actually called. This might reasonably be perceived as overkill. :)


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp:72
+    { // https://bugs.llvm.org/show_bug.cgi?id=27566
+      std::shared_ptr<Result()> x(&theFunction, &resultDeletor);
+    }
----------------
Presumably this should also compile without the explicit `&`s, right? Might be worth adding

    std::shared_ptr<Result()> y(theFunction, resultDeleter);

in addition to your existing test case, just to make sure that omitting the `&` doesn't mess up the template deduction somehow.
IIUC, it would never make sense to write something like

    auto z = std::make_shared<Result()>(theFunction);

so that part doesn't need testing, right?


https://reviews.llvm.org/D30837