[libcxx-commits] [PATCH] D81417: [libcxx] Fix LWG 2874: Constructor shared_ptr::shared_ptr(Y*) should be constrained.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Nitpick: You've got the wrong LWG number in your commit message.

Also, can you please mark the LWG issue as complete in the docs?



================
Comment at: libcxx/include/memory:3565
+                              ((is_array<_Tp>::value && __well_formed_delete_arr_op<_Yp*>::value) ||
+                               (!is_array<_Tp>::value && __well_formed_delete_op<_Yp*>::value));
+};
----------------
You are eagerly instantiating all templates here, and I don't think that's what you want. For example, you'll instantiate `__well_formed_delete_op<_Yp*>` even when `is_array<_Tp>::value` is true.


================
Comment at: libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_Y.pass.cpp:122
+// This should work in C++03 but we get errors when trying to do SFINAE with the delete operator.
+#if STD_TEST_VERSION >= 11
+    {
----------------
`STD_TEST_VERSION` doesn't exist, it's `TEST_STD_VER`.

To catch these sorts of errors, it can be good to add tests and witness their failure before actually fixing them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81417/new/

https://reviews.llvm.org/D81417