More on adding sized deallocation functions in libc++

[lvoufo]

Summary

Continuing from r229281, this adds version guards and test cases.

[lvoufo]

This extends D7799, excluding the _LIBCPP_BUILDING_NEW guard.

[lvoufo]

Adding _LIBCPP_BUILDING_NEW guards.

[lvoufo]

Fix a typo in new.cpp

[rsmith]

Remove the _LIBCPP_BUILDING_NEW guard. If people want the useless C++14 declarations, they can build in C++14 mode...

src/new.cpp

134

Remove this #if/#endif pair and define the weak version unconditionally. The contents of the standard library should not depend on the -std= flag passed when bulding it. Because this function is weak, this won't break valid C++11 and earlier code that happens to be defining such a function.

164

Likewise.

test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp

54

This is not guaranteed to work; your calls to operator new and operator delete are elidable. Directly call operator delete here. (That also lets you run this test in all language modes, not just C++14 mode.)

test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array_nothrow.pass.cpp

30

It's questionable to assume that memory allocated by libc++'s operator new can be freed by std::free. I would suggest either replacing operator new[] too, or just leaking the memory.

[mclow.lists]

src/new.cpp

134

Exactly correct. There should be no requirement to coordinate the -std= flag used to build the library and the flag used to build programs that use the library.

test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array_nothrow.pass.cpp

30

I would prefer replacing operator new[] as well.

49

Shouldn't this be A* ap = new (std::nothrow) A [3]; ??

[lvoufo]

I am uploading an update in a few minutes...

src/new.cpp

134

Done.

134

Ok.

164

Done.

test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp

54

Ok, on the extension of the test in all language modes.
As far as guaranteeing that this works, I would like to keep these passes
(1) consistent with the current design (in {new, new_nothrow, new_array, new_array_nothrow}.pass.cpp), and
(2) more focused on changes in delete than in new,
(3) as resulting from using new/delete expressions rather than new/delete operator function calls (which, as confirmed by some quick tests, happen to follow different lookup paths).
So, I think I'll just define a new handler similarly to the passes in (1).

This incidentally highlights something we may have missed in the standard on sized nothrow deallocation (see FIXMEs on related test cases below).

test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array_nothrow.pass.cpp

30

For the reasons I gave above, I would rather provide a new handler, similarly to the current tests' design, than replace new.

49

Right. I find it strange that the libcxx test system did not catch this error, and several others below. I actually took a closer and replaced the value of 'config.std' in my test/lit.site.cfg file from "c++11" to "c++1y".
Then, running make check-libcxx with this change led to the following unexpected failures.
I wonder if these might happen to be false negatives somehow... How do we normally test "c++14" mode in libcxx?

Failing Tests (11):

libc++ :: std/experimental/string.view/string.view.find/find_last_not_of_char_size.pass.cpp
libc++ :: std/experimental/string.view/string.view.find/find_last_not_of_pointer_size.pass.cpp
libc++ :: std/experimental/string.view/string.view.find/find_last_not_of_pointer_size_size.pass.cpp
libc++ :: std/experimental/string.view/string.view.find/find_last_of_pointer_size.pass.cpp
libc++ :: std/experimental/string.view/string.view.find/find_last_of_pointer_size_size.pass.cpp
libc++ :: std/experimental/string.view/string.view.find/rfind_pointer_size.pass.cpp
libc++ :: std/experimental/string.view/string.view.find/rfind_pointer_size_size.pass.cpp
libc++ :: std/experimental/string.view/string.view.find/rfind_string_view_size.pass.cpp
libc++ :: std/experimental/string.view/string.view.ops/compare.pointer_size.pass.cpp
libc++ :: std/experimental/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp
libc++ :: std/experimental/utilities/time/header.chrono.synop/treat_as_floating_point_v.pass.cpp

[lvoufo]

Updated with better tests and based on previous reviewers' comments.

[lvoufo]

I need to push this in for now, for some time-pressing tests.
This should be safe since it is passing all regression tests and not breaking anything.
We can always roll back later if anything else comes up.

[lvoufo]

In r229968.

include/new

 ;
 _LIBCPP_NEW_DELETE_VIS void* operator new(std::size_t __sz, const std::nothrow_t&) _NOEXCEPT _NOALIAS;
 _LIBCPP_NEW_DELETE_VIS void  operator delete(void* __p) _NOEXCEPT;
+_LIBCPP_NEW_DELETE_VIS void  operator delete(void* __p, const std::nothrow_t&) _NOEXCEPT;
+#if defined(_LIBCPP_BUILDING_NEW) || _LIBCPP_STD_VER >= 14
 _LIBCPP_NEW_DELETE_VIS void  operator delete(void* __p, std::size_t __sz) _NOEXCEPT;
-_LIBCPP_NEW_DELETE_VIS void  operator delete(void* __p, const std::nothrow_t&) _NOEXCEPT;
 _LIBCPP_NEW_DELETE_VIS void  operator delete(void* __p, std::size_t __sz, const std::nothrow_t&) _NOEXCEPT;
+#endif
 
 _LIBCPP_NEW_DELETE_VIS void* operator new[](std::size_t __sz)
 #if !__has_feature(cxx_noexcept)

 ;
 _LIBCPP_NEW_DELETE_VIS void* operator new[](std::size_t __sz, const std::nothrow_t&) _NOEXCEPT _NOALIAS;
 _LIBCPP_NEW_DELETE_VIS void  operator delete[](void* __p) _NOEXCEPT;
+_LIBCPP_NEW_DELETE_VIS void  operator delete[](void* __p, const std::nothrow_t&) _NOEXCEPT;
+#if defined(_LIBCPP_BUILDING_NEW) || _LIBCPP_STD_VER >= 14
 _LIBCPP_NEW_DELETE_VIS void  operator delete[](void* __p, std::size_t __sz) _NOEXCEPT;
-_LIBCPP_NEW_DELETE_VIS void  operator delete[](void* __p, const std::nothrow_t&) _NOEXCEPT;
 _LIBCPP_NEW_DELETE_VIS void  operator delete[](void* __p, std::size_t __sz, const std::nothrow_t&) _NOEXCEPT;
+#endif
 
 inline _LIBCPP_INLINE_VISIBILITY void* operator new  (std::size_t, void* __p) _NOEXCEPT {return __p;}
 inline _LIBCPP_INLINE_VISIBILITY void* operator new[](std::size_t, void* __p) _NOEXCEPT {return __p;}

src/new.cpp

 
 _LIBCPP_WEAK _LIBCPP_NEW_DELETE_VIS
 void
-operator delete(void* ptr, size_t) _NOEXCEPT
+operator delete(void* ptr, const std::nothrow_t&) _NOEXCEPT
 {
     ::operator delete(ptr);
 }
 
+#if _LIBCPP_STD_VER >= 14
 _LIBCPP_WEAK _LIBCPP_NEW_DELETE_VIS
 void
-operator delete(void* ptr, const std::nothrow_t&) _NOEXCEPT
+operator delete(void* ptr, size_t) _NOEXCEPT
 {
     ::operator delete(ptr);
 }
 
 _LIBCPP_WEAK _LIBCPP_NEW_DELETE_VIS
 void
-operator delete(void* ptr, size_t, const std::nothrow_t&) _NOEXCEPT
+operator delete(void* ptr, size_t, const std::nothrow_t& nt) _NOEXCEPT
 {
-    ::operator delete(ptr);
+    ::operator delete(ptr, nt);
 }
+#endif  // _LIBCPP_STD_VER >= 14
 
 _LIBCPP_WEAK _LIBCPP_NEW_DELETE_VIS
 void

 
 _LIBCPP_WEAK _LIBCPP_NEW_DELETE_VIS
 void
-operator delete[] (void* ptr, size_t) _NOEXCEPT
+operator delete[] (void* ptr, const std::nothrow_t&) _NOEXCEPT
 {
     ::operator delete[](ptr);
 }
 
+#if _LIBCPP_STD_VER >= 14
 _LIBCPP_WEAK _LIBCPP_NEW_DELETE_VIS
 void
-operator delete[] (void* ptr, const std::nothrow_t&) _NOEXCEPT
+operator delete[] (void* ptr, size_t) _NOEXCEPT
 {
     ::operator delete[](ptr);
 }
 
 _LIBCPP_WEAK _LIBCPP_NEW_DELETE_VIS
 void
-operator delete[] (void* ptr, size_t, const std::nothrow_t&) _NOEXCEPT
+operator delete[] (void* ptr, size_t, const std::nothrow_t&, nt) _NOEXCEPT
 {
-    ::operator delete[](ptr);
+    ::operator delete[](ptr, nt);
 }
+#endif // _LIBCPP_STD_VER >= 14
 
 #endif // !__GLIBCXX__
 

test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// test sized operator delete[] by replacing unsized operator delete[]
+
+// UNSUPPORTED: asan, msan
+
+#include <new>
+#include <cstddef>
+#include <cstdlib>
+#include <cassert>
+#include <limits>
+
+#if _LIBCPP_STD_VER >= 14
+
+int delete_called = 0;
+int delete_nothrow_called = 0;
+
+void  operator delete[](void* p) throw()
+{
+  ++delete_called;
+  delete_nothrow_called;
+  std::free(p);
+}
+
+void  operator delete[](void* p, const std::nothrow_t&) throw()
+{
+  ++delete_nothrow_called;
+  std::free(p);
+}
+
+bool A_constructed = false;
+
+struct A
+{
+    A() {A_constructed = true;}
+    ~A() {A_constructed = false;}
+};
+
+int main()
+{
+    A* ap = new A[3];
+    assert(ap);
+    assert(A_constructed == 3);
+    assert(!delete_called);
+    assert(!delete_nothrow_called);
+    delete [] ap;
+    assert(!A_constructed);
+    assert(delete_called == 1);
+    assert(!delete_nothrow_called);
+}
+
+#else
+
+int main () {}
+
+#endif // _LIBCPP_STD_VER >= 14

test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array_nothrow.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// test nothrow sized operator delete[] by replacing
+// nothrow unsized operator delete[].
+
+// UNSUPPORTED: asan, msan
+
+#include <new>
+#include <cstddef>
+#include <cstdlib>
+#include <cassert>
+#include <limits>
+
+#if _LIBCPP_STD_VER >= 14
+
+int delete_called = 0;
+int delete_nothrow_called = 0;
+
+void  operator delete[](void* p) throw()
+{
+  ++delete_called;
+  std::free(p);
+}
+
+void  operator delete[](void* p, const std::nothrow_t&) throw()
+{
+  ++delete_nothrow_called;
+  std::free(p);
+}
+
+bool A_constructed = false;
+
+struct A
+{
+    A() {A_constructed = true;}
+    ~A() {A_constructed = false;}
+};
+
+int main()
+{
+    A* ap = new A(std::nothrow) [3];
+    assert(ap);
+    assert(A_constructed == 3);
+    assert(!delete_called);
+    assert(!delete_nothrow_called);
+    delete [] ap;
+    assert(!A_constructed);
+    assert(!delete_called);
+    assert(delete_nothrow_called == 1);
+}
+
+#else
+
+int main () {}
+
+#endif // _LIBCPP_STD_VER >= 14

test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array_nothrow_replace.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// test nothrow sized operator delete[] replacement
+
+// UNSUPPORTED: asan, msan
+
+#include <new>
+#include <cstddef>
+#include <cstdlib>
+#include <cassert>
+#include <limits>
+
+#if _LIBCPP_STD_VER >= 14
+
+int unsized_delete_called = 0;
+int unsized_delete_nothrow_called = 0;
+int sized_delete_called = 0;
+int sized_delete_nothrow_called = 0;
+
+void  operator delete[](void* p) throw()
+{
+  ++unsized_delete_called;
+  std::free(p);
+}
+
+void  operator delete[](void* p, const std::nothrow_t&) throw()
+{
+  ++unsized_delete_nothrow_called;
+  std::free(p);
+}
+
+void  operator delete[](void* p, std::size_t) throw()
+{
+    ++size_delete_called;
+    std::free(p);
+}
+
+void  operator delete[](void* p, std::size_t, const std::nothrow_t&) throw()
+{
+  ++size_delete_nothrow_called;
+  std::free(p);
+}
+
+bool A_constructed = false;
+
+struct A
+{
+    A() {A_constructed = true;}
+    ~A() {A_constructed = false;}
+};
+
+int main()
+{
+    A* ap = new A(std::nothrow) [3];
+    assert(ap);
+    assert(A_constructed);
+    assert(!unsized_delete_called);
+    assert(!unsized_delete_nothrow_called);
+    assert(!sized_delete_called);
+    assert(!sized_delete_nothrow_called);
+    delete [] ap;
+    assert(!A_constructed);
+    assert(!unsized_delete_called);
+    assert(!unsized_delete_nothrow_called);
+    assert(!sized_delete_called);
+    assert(sized_delete_nothrow_called == 1);
+}
+
+#else
+
+int main () {}
+
+#endif // _LIBCPP_STD_VER >= 14

test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array_replace.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// test sized operator delete[] replacement
+
+// UNSUPPORTED: asan, msan
+
+#include <new>
+#include <cstddef>
+#include <cstdlib>
+#include <cassert>
+#include <limits>
+
+#if _LIBCPP_STD_VER >= 14
+
+int unsized_delete_called = 0;
+int unsized_delete_nothrow_called = 0;
+int sized_delete_called = 0;
+int sized_delete_nothrow_called = 0;
+
+void  operator delete[](void* p) throw()
+{
+  ++unsized_delete_called;
+  std::free(p);
+}
+
+void  operator delete[](void* p, const std::nothrow_t&) throw()
+{
+  ++unsized_delete_nothrow_called;
+  std::free(p);
+}
+
+void  operator delete[](void* p, std::size_t) throw()
+{
+    ++size_delete_called;
+    std::free(p);
+}
+
+void  operator delete[](void* p, std::size_t, const std::nothrow_t&) throw()
+{
+  ++size_delete_nothrow_called;
+  std::free(p);
+}
+
+bool A_constructed = false;
+
+struct A
+{
+    A() {A_constructed = true;}
+    ~A() {A_constructed = false;}
+};
+
+int main()
+{
+    A* ap = new A[3];
+    assert(ap);
+    assert(A_constructed == 3);
+    assert(!unsized_delete_called);
+    assert(!unsized_delete_nothrow_called);
+    assert(!sized_delete_called);
+    assert(!sized_delete_nothrow_called);
+    delete [] ap;
+    assert(!A_constructed);
+    assert(!unsized_delete_called);
+    assert(!unsized_delete_nothrow_called);
+    assert(sized_delete_called == 1);
+    assert(!sized_delete_nothrow_called);
+}
+
+#else
+
+int main () {}
+
+#endif // _LIBCPP_STD_VER >= 14

test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// test sized operator delete by replacing unsized operator delete
+
+// UNSUPPORTED: asan, msan
+
+#include <new>
+#include <cstddef>
+#include <cstdlib>
+#include <cassert>
+#include <limits>
+
+#if _LIBCPP_STD_VER >= 14
+
+int delete_called = 0;
+int delete_nothrow_called = 0;
+
+void  operator delete(void* p) throw()
+{
+  ++delete_called;
+  delete_nothrow_called;
+  std::free(p);
+}
+
+void  operator delete(void* p, const std::nothrow_t&) throw()
+{
+  ++delete_nothrow_called;
+  std::free(p);
+}
+
+bool A_constructed = false;
+
+struct A
+{
+    A() {A_constructed = true;}
+    ~A() {A_constructed = false;}
+};
+
+int main()
+{
+    A* ap = new A;
+    assert(ap);
+    assert(A_constructed);
+    assert(!delete_called);
+    assert(!delete_nothrow_called);
+    delete ap;
+    assert(!A_constructed);
+    assert(delete_called == 1);
+    assert(!delete_nothrow_called);
+}
+
+#else
+
+int main () {}
+
+#endif // _LIBCPP_STD_VER >= 14

test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete_nothrow.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// test nothrow sized operator delete by replacing
+// nothrow unsized operator delete.
+
+// UNSUPPORTED: asan, msan
+
+#include <new>
+#include <cstddef>
+#include <cstdlib>
+#include <cassert>
+#include <limits>
+
+#if _LIBCPP_STD_VER >= 14
+
+int delete_called = 0;
+int delete_nothrow_called = 0;
+
+void  operator delete(void* p) throw()
+{
+  ++delete_called;
+  delete_nothrow_called;
+  std::free(p);
+}
+
+void  operator delete(void* p, const std::nothrow_t&) throw()
+{
+  ++delete_nothrow_called;
+  std::free(p);
+}
+
+bool A_constructed = false;
+
+struct A
+{
+    A() {A_constructed = true;}
+    ~A() {A_constructed = false;}
+};
+
+int main()
+{
+    A* ap = new A(std::nothrow);
+    assert(ap);
+    assert(A_constructed);
+    assert(!delete_called);
+    assert(!delete_nothrow_called);
+    delete ap;
+    assert(!A_constructed);
+    assert(!delete_called);
+    assert(delete_nothrow_called == 1);
+}
+
+#else
+
+int main () {}
+
+#endif // _LIBCPP_STD_VER >= 14

test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete_nothrow_replace.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// test nothrow sized operator delete replacement
+
+// UNSUPPORTED: asan, msan
+
+#include <new>
+#include <cstddef>
+#include <cstdlib>
+#include <cassert>
+#include <limits>
+
+#if _LIBCPP_STD_VER >= 14
+
+int unsized_delete_called = 0;
+int unsized_delete_nothrow_called = 0;
+int sized_delete_called = 0;
+int sized_delete_nothrow_called = 0;
+
+void  operator delete(void* p) throw()
+{
+  ++unsized_delete_called;
+  std::free(p);
+}
+
+void  operator delete(void* p, const std::nothrow_t&) throw()
+{
+  ++unsized_delete_nothrow_called;
+  std::free(p);
+}
+
+void  operator delete(void* p, std::size_t) throw()
+{
+    ++size_delete_called;
+    std::free(p);
+}
+
+void  operator delete(void* p, std::size_t, const std::nothrow_t&) throw()
+{
+  ++size_delete_nothrow_called;
+  std::free(p);
+}
+
+bool A_constructed = false;
+
+struct A
+{
+    A() {A_constructed = true;}
+    ~A() {A_constructed = false;}
+};
+
+int main()
+{
+    A* ap = new A(std::nothrow);
+    assert(ap);
+    assert(A_constructed);
+    assert(!unsized_delete_called);
+    assert(!unsized_delete_nothrow_called);
+    assert(!sized_delete_called);
+    assert(!sized_delete_nothrow_called);
+    delete ap;
+    assert(!A_constructed);
+    assert(!unsized_delete_called);
+    assert(!unsized_delete_nothrow_called);
+    assert(!sized_delete_called);
+    assert(sized_delete_nothrow_called == 1);
+}
+
+#else
+
+int main () {}
+
+#endif // _LIBCPP_STD_VER >= 14

test/std/language.support/support.dynamic/new.delete/new.delete.single/sized_delete_replace.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// test sized operator delete replacement
+
+// UNSUPPORTED: asan, msan
+
+#include <new>
+#include <cstddef>
+#include <cstdlib>
+#include <cassert>
+#include <limits>
+
+#if _LIBCPP_STD_VER >= 14
+
+int unsized_delete_called = 0;
+int unsized_delete_nothrow_called = 0;
+int sized_delete_called = 0;
+int sized_delete_nothrow_called = 0;
+
+void  operator delete(void* p) throw()
+{
+  ++unsized_delete_called;
+  std::free(p);
+}
+
+void  operator delete(void* p, const std::nothrow_t&) throw()
+{
+  ++unsized_delete_nothrow_called;
+  std::free(p);
+}
+
+void  operator delete(void* p, std::size_t) throw()
+{
+    ++size_delete_called;
+    std::free(p);
+}
+
+void  operator delete(void* p, std::size_t, const std::nothrow_t&) throw()
+{
+  ++size_delete_nothrow_called;
+  std::free(p);
+}
+
+bool A_constructed = false;
+
+struct A
+{
+    A() {A_constructed = true;}
+    ~A() {A_constructed = false;}
+};
+
+int main()
+{
+    A* ap = new A;
+    assert(ap);
+    assert(A_constructed);
+    assert(!unsized_delete_called);
+    assert(!unsized_delete_nothrow_called);
+    assert(!sized_delete_called);
+    assert(!sized_delete_nothrow_called);
+    delete ap;
+    assert(!A_constructed);
+    assert(!unsized_delete_called);
+    assert(!unsized_delete_nothrow_called);
+    assert(sized_delete_called == 1);
+    assert(!sized_delete_nothrow_called);
+}
+
+#else
+
+int main () {}
+
+#endif // _LIBCPP_STD_VER >= 14