Re: [v3 PATCH] Implement std::is_aggregate.

[Jakub Jelinek <jakub@redhat.com>]

On Sun, Apr 02, 2017 at 01:18:58AM +0300, Ville Voutilainen wrote:
> Tested on Linux-x64.
> 
> 2017-04-02  Ville Voutilainen  <ville.voutilainen@gmail.com>
> 
>     Implement std::is_aggregate.
>     * include/std/type_traits (is_aggregate): New.
>     * testsuite/20_util/is_aggregate/requirements/explicit_instantiation.cc:
>     New.
>     * testsuite/20_util/is_aggregate/requirements/typedefs.cc: Likewise.
>     * testsuite/20_util/is_aggregate/value.cc: Likewise.
>
> diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
> index 6707caa..89a3738 100644
> --- a/libstdc++-v3/include/std/type_traits
> +++ b/libstdc++-v3/include/std/type_traits
> @@ -3062,6 +3062,25 @@ template <typename _From, typename _To>
>  #endif
>  #undef _GLIBCXX_NO_BUILTIN_HAS_UNIQ_OBJ_REP
>  
> +#ifdef __has_builtin
> +# if !__has_builtin(__is_aggregate)
> +// Try not to break non-GNU compilers that don't support the built-in:
> +#  define _GLIBCXX_NO_BUILTIN_IS_AGGREGATE 1
> +# endif
> +#endif
> +
> +#ifndef _GLIBCXX_NO_BUILTIN_IS_AGGREGATE
> +#define __cpp_lib_is_aggregate 201703
> +  /// is_aggregate
> +  template<typename _Tp>
> +    struct is_aggregate
> +    : bool_constant<__is_aggregate(
> +      remove_cv_t<_Tp>
> +      )>

Any reason for the wrapping?

> +    : bool_constant<__is_aggregate(remove_cv_t<_Tp>)>

Would fit on one line.

I admit I know next to nothing about libstdc++ code formatting.

> +    { };

Also, shouldn't there be also:

  /// is_aggregate_v
  template<typename _Tp>
    _GLIBCXX17_INLINE constexpr bool is_aggregate_v =
      is_aggregate<_Tp>::value;

somewhere with appropriate guards (or within the same ones)?

> +#endif
> +#undef _GLIBCXX_NO_BUILTIN_IS_AGGREGATE
> +  
>  #endif // C++17
>  
>  _GLIBCXX_END_NAMESPACE_VERSION

I'm surprised tests for the is_*_v variable templates are only in
experimental/type_traits/value.cc when they are now apparently
part of C++17.

	Jakub