[libcxx-commits] [PATCH] D62103: Update shared_ptr tests to match the standard

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added inline comments.


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/types.pass.cpp:39
 {
-    static_assert((std::is_same<std::shared_ptr<A>::element_type, A>::value), "");
+    static_assert(std::is_same<typename std::shared_ptr<T>::element_type, T>::value);
 #if TEST_STD_VER > 14
----------------
Two points here. 

In C++03, we have an emulation of `static_assert` that is a macro. Sadly, like all macros, it gets confused by commas. "Is this a comma in the expression, or another macro parameter?" Hence, the extra parenthesis which you removed. Please put them back.

Also, why not use `ASSERT_SAME_TYPE`?



================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/default.pass.cpp:27
+    {
+        std::shared_ptr<T> p {};
+        assert(p.use_count() == 0);
----------------
Why braces here? 

You might want to read up on the difference between default-initialization and value-initialization.



================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/pointer.pass.cpp:37
     assert(A::count == 0);
+
     {
----------------
Is "whitespace" the only change to this file?


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/pointer_deleter.pass.cpp:39
+
+        test_deleter<A>* d = std::get_deleter<test_deleter<A>>(p);
+        assert(test_deleter<A>::count == 1);
----------------
By removing this space, you make it so it doesn't work in C++03.


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/pointer_deleter_allocator.pass.cpp:72
     test_deleter<A>::dealloc_count = 0;
-#if TEST_STD_VER >= 11
+
     // Test an allocator that returns class-type pointers
----------------
`min_allocator` is a C++11 test thing.



================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_Y_rv.pass.cpp:72
             assert(B::count == 1);
             assert(A::count == 1);
             assert(pB.use_count() == 1);
----------------
I don't see any explanation as to why you're stripping out these C++03 tests.


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_Y_rv.pass.cpp:96
             assert(pA.get() == pB.get());
+            assert(pA.get() == nullptr);
         }
----------------
I would think that this test should go on line 87


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_copy_move.fail.cpp:42
+
+#if TEST_STD_VER > 14
+    {
----------------
Why is this bit C++14 only?


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/shared_ptr_pointer.pass.cpp:73
+
+#if TEST_STD_VER > 14
+    {
----------------
Again, why `14`?


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/unique_ptr.pass.cpp:10
+// UNSUPPORTED: sanitizer-new-delete, c++98, c++03
+
 
----------------
Why disable this entire test for C++98/03?


================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp:83
     std::shared_ptr<A> p = std::make_shared<A>(i, c);
-    assert(globalMemCounter.checkOutstandingNewEq(nc+1));
+    assert(globalMemCounter.checkOutstandingNewEq(nc+1)); // one for make_shared (create type ptr) and one for creation of control block.
     assert(A::count == 1);
----------------
This comment is confusing to me. 
It checks for `+1`, but the comment implies //two//.

(There should be only one allocation, btw).



================
Comment at: test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.obs/op_bool.pass.cpp:39
+
+        if (std::shared_ptr<B> sp = std::dynamic_pointer_cast<B>(basePtr))
+        { check = true; }
----------------
This seems convoluted. Why not:
```
std::shared_ptr<B> sp = std::dynamic_pointer_cast<B>(basePtr);
assert(sp);
```


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

https://reviews.llvm.org/D62103