On Feb 9, 2014, at 12:03 PM, Marshall Clow <[email protected]> wrote:

> On the CFE-dev list, Kal reported:
> 
>> Running libc++ 3.4 rc1 "testit" on 32-bit Linux fails for test:
>> 
>> test/language.support/support.types/max_align_t.pass.cpp
>> 
>> max_align_t is typedef'd to "long double" type in <cstddef>. But...
>> 
>> alignment_of(long double)=4, sizeof(long double)=12
>> alignment_of(long long)=8, sizeof(long long)=8
> 
> and last night, in r201037, David Majnemer added support for a macro 
> __ALIGNOF_MAX_ALIGN_T__.
> 
> This patch changes the definition of max_align_t when that macro is present.
> 
> Note: The actual definition of max_align_t is:
> • The type max_align_t is a POD type whose alignment requirement is at least 
> as great as that of every scalar type, and whose alignment requirement is 
> supported in every context.
> 
> — Marshall
> 
> <max_align.patch>

Thanks Kal and Marshall!

I think this is a good patch.  However I have a counter-proposal that is a bit 
more ambitious:

This patch moves the private __find_max_align utility from <type_traits> to 
<cstddef> (<type_traits> includes <cstddef>).  This utility creates a list of 
likely types, and their alignments, and then given a sizeof, it selects an 
appropriate alignment.  This is used in the implementation of 
std::aligned_storage_t for the defaulted alignment parameter.

The purpose of moving this utility is to fall back on it in case we can't find 
max_align_t in <stddef.h>, and in case __ALIGNOF_MAX_ALIGN_T__ is not already 
defined by the compiler (or I supposed by <__config> for some platform).  This 
is simply done by getting the alignment for a ridiculously large type, say 
sizeof == 1Kb:

#ifndef __ALIGNOF_MAX_ALIGN_T__
#define __ALIGNOF_MAX_ALIGN_T__ (__find_max_align<__all_types, 1024>::value)
#endif

On OSX / iOS, __find_max_align<__all_types, 1024>::value == 16.  If some 
platform needs to add a type to the type list __all_types, that is easily done. 
 But this facility has been used in aligned_storage for years, and no one has 
yet had any problems with it.

Features:

1. If <__config> defines _LIBCPP_C_HAS_MAX_ALIGN_T, then max_align_t is grabbed 
from the global namespace / <stddef.h>.  Currently no platform in <__config> 
defines _LIBCPP_C_HAS_MAX_ALIGN_T.  C11 introduces max_align_t in <stddef.h>.  
I expect that eventually all platforms will migrate to this option.

2. If _LIBCPP_C_HAS_MAX_ALIGN_T is not defined, but __ALIGNOF_MAX_ALIGN_T__ is 
defined, then max_align_t is created as proposed by Marshall.  As Marshall 
noted, Kal has just added __ALIGNOF_MAX_ALIGN_T__ to the tip-of-trunk clang.

3. If neither _LIBCPP_C_HAS_MAX_ALIGN_T nor __ALIGNOF_MAX_ALIGN_T__ is defined, 
then __ALIGNOF_MAX_ALIGN_T__ is defined as (__find_max_align<__all_types, 
1024>::value), and then again max_align_t is created as proposed by Marshall.  
This latter step is I believe a better default than what we originally had:  
long double.

All of the code dealing with alignment is pretty ancient in libc++, and it was 
overdue for an update.  Along the way I've modernized and simplified 
aligned_storage.  And there's a drive-by fix of a C++03 bug in common_type<int> 
pointed out by Marshall.  And another drive-by fix of a C++03 warning in 
test/utilities/meta/meta.unary/meta.unary.prop/is_assignable.pass.cpp.

I've tested with Apple's clang-500.0.68 (which lacks __ALIGNOF_MAX_ALIGN_T__), 
in C++03 and C++11, and with tip-of-trunk clang (which has 
__ALIGNOF_MAX_ALIGN_T__) in C++03, C++11 and C++1y.  The branch which imports 
max_align_t from <stddef.h> has not been tested.  But this branch consists of 
only a single line of code:

using ::max_align_t;

Howard

Attachment: max_align_t.patch
Description: Binary data

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to