joerg marked an inline comment as done.
joerg added inline comments.
================
Comment at: libcxx/include/new:229-237
+#if !defined(_LIBCPP_CXX03_LANG)
+using __libcpp_max_align_t = max_align_t;
+#else
+union __libcpp_max_align_t {
+ void * __f1;
+ long long int __f2;
+ long double __f3;
----------------
EricWF wrote:
> rsmith wrote:
> > Is there any ODR risk from this, or similar? Does libc++ support building
> > in mixed C++98 / C++11 mode? If different TUs disagree on this alignment,
> > we can end up allocating with the aligned allocator and deallocating with
> > the unaligned allocator, which is not guaranteed to work.
> >
> > We could always use the union approach if we don't know the default new
> > alignment. But from the code below it looks like we might only ever use
> > this if we have aligned allocation support, in which case we can just
> > assume that the default new alignment is defined. So perhaps we can just
> > hide the entire definition of `__is_overaligned_for_new` behind a `#ifdef
> > __STDCPP_DEFAULT_NEW_ALIGNMENT__` and never even consider `max_align_t`?
> `max_align_t` should be ABI stable. I would rather we copy/paste the clang
> definition into the libc++ sources so we can use it when it's not provided by
> the compiler.
>
> Though this raises another can of worms because GCC and Clang don't agree on
> a size for `max_align_t`.
max_align_t doesn't exist in C++03 mode, the included version is the nearest
thing possible in portable C++. A visible difference happens if:
(1) The user requires C++03
(2) The code contains non-standard types with either explicit alignment or
larger implicit alignment than the base types.
(3) __STDCPP_DEFAULT_NEW_ALIGNMENT__ or alignof(max_align_t) is larger than the
alignment of the union.
In that case, behavior changes as to which allocator/deallocator is used. If
the explicit aligned version is not compatible and life-time crosses into c++11
mode, it could be a problem. But at that point I think we did all we could
possible do to provide compatibility and the code is too far in
implementation-defined land already.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73245/new/
https://reviews.llvm.org/D73245
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
