[libc++] Move the definition of aligned allocation helpers outside of <new>

[ldionne]

Summary

They are not needed in <new> -- in fact they are only needed in .cpp files.
Getting those out of the way makes the headers smaller and also makes it
easier to use the library on platforms where aligned allocation is not
available.

[ldionne]

Try to fix CI

[Mordante]

libcxx/include/__memory/aligned_alloc.h

11

Why is the header not included by __memory? When intended it would be good to document why it's the case.
Then the guarded include in the .cpp files can be removed too.

20

Shouldn't this code be guarded by _LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION?

[ldionne]

Rebase and address some comments

libcxx/include/__memory/aligned_alloc.h

11

Like we do for many other implementation-detail headers, we don't include them in the main public header unless they provide something that is part of the public. We actually have the opposite policy (to include everything, as you mention), but we do it very inconsistently, sometimes because it's literally impossible to do otherwise. Hence, I think we should shift the policy towards including only the things that are necessary for the public API.

[ldionne]

Try to fix CI

[ldionne]

Both CI failures are flakes.

[Mordante]

libcxx/include/__memory/aligned_alloc.h

11

I think it would be good to document our policy. I sometimes get the impression different developers have different ideas what the policies are.

libcxx/include/CMakeLists.txt

set(files
   __iterator/size.h
   __iterator/sortable.h
   __iterator/unreachable_sentinel.h
   __iterator/wrap_iter.h
   __locale
   __mbstate_t.h
   __memory/addressof.h
   __memory/align.h
+  __memory/aligned_alloc.h
   __memory/allocate_at_least.h
   __memory/allocation_guard.h
   __memory/allocator.h
   __memory/allocator_arg_t.h
   __memory/allocator_destructor.h
   __memory/allocator_traits.h
   __memory/assume_aligned.h
   __memory/auto_ptr.h

libcxx/include/__memory/aligned_alloc.h

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___MEMORY_ALIGNED_ALLOC_H
+#define _LIBCPP___MEMORY_ALIGNED_ALLOC_H
+
+#include <__config>
+#include <cstddef>
+#include <cstdlib>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#ifndef _LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION
+
+// Low-level helpers to call the aligned allocation and deallocation functions
+// on the target platform. This is used to implement libc++'s own memory
+// allocation routines -- if you need to allocate memory inside the library,
+// chances are that you want to use `__libcpp_allocate` instead.
+//
+// Returns the allocated memory, or `nullptr` on failure.
+inline _LIBCPP_HIDE_FROM_ABI
+void* __libcpp_aligned_alloc(std::size_t __alignment, std::size_t __size) {
+#  if defined(_LIBCPP_MSVCRT_LIKE)
+    return ::_aligned_malloc(__size, __alignment);
+#  elif _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_C11_ALIGNED_ALLOC)
+    // aligned_alloc() requires that __size is a multiple of __alignment,
+    // but for C++ [new.delete.general], only states "if the value of an
+    // alignment argument passed to any of these functions is not a valid
+    // alignment value, the behavior is undefined".
+    // To handle calls such as ::operator new(1, std::align_val_t(128)), we
+    // round __size up to the next multiple of __alignment.
+    size_t __rounded_size = (__size + __alignment - 1) & ~(__alignment - 1);
+    // Rounding up could have wrapped around to zero, so we have to add another
+    // max() ternary to the actual call site to avoid succeeded in that case.
+    return ::aligned_alloc(__alignment, __size > __rounded_size ? __size : __rounded_size);
+#  else
+    void* __result = nullptr;
+    (void)::posix_memalign(&__result, __alignment, __size);
+    // If posix_memalign fails, __result is unmodified so we still return `nullptr`.
+    return __result;
+#  endif
+}
+
+inline _LIBCPP_HIDE_FROM_ABI
+void __libcpp_aligned_free(void* __ptr) {
+#if defined(_LIBCPP_MSVCRT_LIKE)
+  ::_aligned_free(__ptr);
+#else
+  ::free(__ptr);
+#endif
+}
+
+#endif // !_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___MEMORY_ALIGNED_ALLOC_H

libcxx/include/module.modulemap.in

   }
   module memory {
     header "memory"
     export *
 
     module __memory {
       module addressof                       { private header "__memory/addressof.h" }
       module align                           { private header "__memory/align.h" }
+      module aligned_alloc                   { private header "__memory/aligned_alloc.h" }
       module allocate_at_least               { private header "__memory/allocate_at_least.h" }
       module allocation_guard                { private header "__memory/allocation_guard.h" }
       module allocator                       { private header "__memory/allocator.h" }
       module allocator_arg_t                 { private header "__memory/allocator_arg_t.h" }
       module allocator_destructor            { private header "__memory/allocator_destructor.h" }
       module allocator_traits                { private header "__memory/allocator_traits.h" }
       module assume_aligned                  { private header "__memory/assume_aligned.h" }
       module auto_ptr                        { private header "__memory/auto_ptr.h" }

libcxx/include/new

if (__is_overaligned_for_new(__align)) {
       const align_val_t __align_val = static_cast<align_val_t>(__align);
       return __libcpp_operator_delete(__ptr, __align_val);
     } else {
       return __libcpp_operator_delete(__ptr);
     }
 #endif
 }
 
-#if !defined(_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION)
-// Low-level helpers to call the aligned allocation and deallocation functions
-// on the target platform. This is used to implement libc++'s own memory
-// allocation routines -- if you need to allocate memory inside the library,
-// chances are that you want to use `__libcpp_allocate` instead.
-//
-// Returns the allocated memory, or `nullptr` on failure.
-inline _LIBCPP_INLINE_VISIBILITY void* __libcpp_aligned_alloc(std::size_t __alignment, std::size_t __size) {
-#  if defined(_LIBCPP_MSVCRT_LIKE)
-    return ::_aligned_malloc(__size, __alignment);
-#  elif _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_C11_ALIGNED_ALLOC)
-    // aligned_alloc() requires that __size is a multiple of __alignment,
-    // but for C++ [new.delete.general], only states "if the value of an
-    // alignment argument passed to any of these functions is not a valid
-    // alignment value, the behavior is undefined".
-    // To handle calls such as ::operator new(1, std::align_val_t(128)), we
-    // round __size up to the next multiple of __alignment.
-    size_t __rounded_size = (__size + __alignment - 1) & ~(__alignment - 1);
-    // Rounding up could have wrapped around to zero, so we have to add another
-    // max() ternary to the actual call site to avoid succeeded in that case.
-    return ::aligned_alloc(__alignment, __size > __rounded_size ? __size : __rounded_size);
-#  else
-    void* __result = nullptr;
-    (void)::posix_memalign(&__result, __alignment, __size);
-    // If posix_memalign fails, __result is unmodified so we still return `nullptr`.
-    return __result;
-#  endif
-}
-
-inline _LIBCPP_INLINE_VISIBILITY
-void __libcpp_aligned_free(void* __ptr) {
-#if defined(_LIBCPP_MSVCRT_LIKE)
-  ::_aligned_free(__ptr);
-#else
-  ::free(__ptr);
-#endif
-}
-#endif // !_LIBCPP_HAS_NO_ALIGNED_ALLOCATION
-
-
 template <class _Tp>
 _LIBCPP_NODISCARD_AFTER_CXX17 inline _LIBCPP_HIDE_FROM_ABI
 _LIBCPP_CONSTEXPR _Tp* __launder(_Tp* __p) _NOEXCEPT
 {
     static_assert (!(is_function<_Tp>::value), "can't launder functions" );
     static_assert (!(is_same<void, __remove_cv_t<_Tp> >::value), "can't launder cv-void" );
     return __builtin_launder(__p);
 }

libcxx/src/new.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include <__memory/aligned_alloc.h>
 #include <new>
 #include <stdlib.h>
 
 #include "include/atomic_support.h"
 
 #if defined(_LIBCPP_ABI_MICROSOFT)
 #   if !defined(_LIBCPP_ABI_VCRUNTIME)
 #       include "support/runtime/new_handler_fallback.ipp"

libcxx/test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp

 
 #include <new>
 #include <typeinfo>
 #include <string>
 #include <cassert>
 
 #include "test_macros.h"
 
+TEST_DIAGNOSTIC_PUSH
+TEST_CLANG_DIAGNOSTIC_IGNORED("-Wprivate-header")
+#include <__memory/aligned_alloc.h>
+TEST_DIAGNOSTIC_POP
+
 struct alloc_stats {
   alloc_stats() { reset(); }
 
   int aligned_sized_called;
   int aligned_called;
   int sized_called;
   int plain_called;
   int last_size;

libcxx/test/libcxx/private_headers.verify.cpp

 #include <__iterator/size.h> // expected-error@*:* {{use of private header from outside its module: '__iterator/size.h'}}
 #include <__iterator/sortable.h> // expected-error@*:* {{use of private header from outside its module: '__iterator/sortable.h'}}
 #include <__iterator/unreachable_sentinel.h> // expected-error@*:* {{use of private header from outside its module: '__iterator/unreachable_sentinel.h'}}
 #include <__iterator/wrap_iter.h> // expected-error@*:* {{use of private header from outside its module: '__iterator/wrap_iter.h'}}
 #include <__locale> // expected-error@*:* {{use of private header from outside its module: '__locale'}}
 #include <__mbstate_t.h> // expected-error@*:* {{use of private header from outside its module: '__mbstate_t.h'}}
 #include <__memory/addressof.h> // expected-error@*:* {{use of private header from outside its module: '__memory/addressof.h'}}
 #include <__memory/align.h> // expected-error@*:* {{use of private header from outside its module: '__memory/align.h'}}
+#include <__memory/aligned_alloc.h> // expected-error@*:* {{use of private header from outside its module: '__memory/aligned_alloc.h'}}
 #include <__memory/allocate_at_least.h> // expected-error@*:* {{use of private header from outside its module: '__memory/allocate_at_least.h'}}
 #include <__memory/allocation_guard.h> // expected-error@*:* {{use of private header from outside its module: '__memory/allocation_guard.h'}}
 #include <__memory/allocator.h> // expected-error@*:* {{use of private header from outside its module: '__memory/allocator.h'}}
 #include <__memory/allocator_arg_t.h> // expected-error@*:* {{use of private header from outside its module: '__memory/allocator_arg_t.h'}}
 #include <__memory/allocator_destructor.h> // expected-error@*:* {{use of private header from outside its module: '__memory/allocator_destructor.h'}}
 #include <__memory/allocator_traits.h> // expected-error@*:* {{use of private header from outside its module: '__memory/allocator_traits.h'}}
 #include <__memory/assume_aligned.h> // expected-error@*:* {{use of private header from outside its module: '__memory/assume_aligned.h'}}
 #include <__memory/auto_ptr.h> // expected-error@*:* {{use of private header from outside its module: '__memory/auto_ptr.h'}}

libcxxabi/src/fallback_malloc.cpp

 
 #include <__threading_support>
 #ifndef _LIBCXXABI_HAS_NO_THREADS
 #if defined(__ELF__) && defined(_LIBCXXABI_LINK_PTHREAD_LIB)
 #pragma comment(lib, "pthread")
 #endif
 #endif
 
+#include <__memory/aligned_alloc.h>
 #include <assert.h>
 #include <stdlib.h> // for malloc, calloc, free
 #include <string.h> // for memset
-#include <new> // for std::__libcpp_aligned_{alloc,free}
 
 //  A small, simple heap manager based (loosely) on
 //  the startup heap manager from FreeBSD, optimized for space.
 //
 //  Manages a fixed-size memory pool, supports malloc and free only.
 //  No support for realloc.
 //
 //  Allocates chunks in multiples of four bytes, with a four byte header

libcxxabi/src/stdlib_new_delete.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //
 // This file implements the new and delete operators.
 //===----------------------------------------------------------------------===//
 
 #include "__cxxabi_config.h"
-#include <new>
+#include <__memory/aligned_alloc.h>
 #include <cstdlib>
+#include <new>
 
 #if !defined(_THROW_BAD_ALLOC) || !defined(_LIBCXXABI_WEAK)
 #error The _THROW_BAD_ALLOC and _LIBCXXABI_WEAK libc++ macros must \
        already be defined by libc++.
 #endif
 // Implement all new and delete operators as weak definitions
 // in this shared library, so that they can be overridden by programs
 // that define non-weak copies of the functions.

libcxxabi/test/test_fallback_malloc.pass.cpp

 // only understands _LIBCXXABI_HAS_NO_THREADS.
 #include "test_macros.h"
 #ifdef TEST_HAS_NO_THREADS
 # define _LIBCXXABI_HAS_NO_THREADS
 #endif
 
 typedef std::deque<void *> container;
 
+TEST_DIAGNOSTIC_PUSH
+TEST_CLANG_DIAGNOSTIC_IGNORED("-Wprivate-header")
 // #define  DEBUG_FALLBACK_MALLOC
 #define INSTRUMENT_FALLBACK_MALLOC
 #include "../src/fallback_malloc.cpp"
+TEST_DIAGNOSTIC_POP
 
 void assertAlignment(void* ptr) { assert(reinterpret_cast<size_t>(ptr) % alignof(FallbackMaxAlignType) == 0); }
 
 container alloc_series ( size_t sz ) {
     container ptrs;
     void *p;
 
     while (NULL != (p = fallback_malloc(sz))) {