On Tue, 24 Jun 2025 at 13:53, Patrick Palka <ppa...@redhat.com> wrote:
>
> On Tue, 24 Jun 2025, Jonathan Wakely wrote:
>
> > On Tue, 24 Jun 2025 at 03:20, Patrick Palka <ppa...@redhat.com> wrote:
> > >
> > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> > >
> > > -- >8 --
> > >
> > > When checking __is_complete_or_unbounded on a reference to incomplete
> > > type, we overeagerly try to instantiate/complete the referenced type
> > > which besides being unnecessary may also produce a -Wsfinae-incomplete
> > > warning (added in r16-1527) if the referenced type is later defined.
> > >
> > > This patch fixes this by effectively restricting the sizeof check to
> > > object (except unknown-bound array) types. In passing simplify the
> > > implementation by using is_object instead of is_function/reference/void.
> > >
> > > PR libstdc++/120717
> > >
> > > libstdc++-v3/ChangeLog:
> > >
> > > * include/std/type_traits (__is_complete_or_unbounded): Don't
> > > check sizeof on a reference or unbounded array type. Simplify
> > > using is_object. Correct formatting.
> > > * testsuite/20_util/is_complete_or_unbounded/120717.cc: New test.
> > > ---
> > > libstdc++-v3/include/std/type_traits | 34 +++++++++----------
> > > .../is_complete_or_unbounded/120717.cc | 20 +++++++++++
> > > 2 files changed, 37 insertions(+), 17 deletions(-)
> > > create mode 100644
> > > libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/120717.cc
> > >
> > > diff --git a/libstdc++-v3/include/std/type_traits
> > > b/libstdc++-v3/include/std/type_traits
> > > index abff9f880001..28960befd2c7 100644
> > > --- a/libstdc++-v3/include/std/type_traits
> > > +++ b/libstdc++-v3/include/std/type_traits
> > > @@ -280,11 +280,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > >
> > > // Forward declarations
> > > template<typename>
> > > - struct is_reference;
> > > - template<typename>
> > > - struct is_function;
> > > - template<typename>
> > > - struct is_void;
> > > + struct is_object;
> > > template<typename>
> > > struct remove_cv;
> > > template<typename>
> > > @@ -297,18 +293,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > // Helper functions that return false_type for incomplete classes,
> > > // incomplete unions and arrays of known bound from those.
> > >
> > > - template <typename _Tp, size_t = sizeof(_Tp)>
> > > - constexpr true_type __is_complete_or_unbounded(__type_identity<_Tp>)
> > > - { return {}; }
> > > -
> > > - template <typename _TypeIdentity,
> > > - typename _NestedType = typename _TypeIdentity::type>
> > > - constexpr typename __or_<
> > > - is_reference<_NestedType>,
> > > - is_function<_NestedType>,
> > > - is_void<_NestedType>,
> > > - __is_array_unknown_bounds<_NestedType>
> > > - >::type __is_complete_or_unbounded(_TypeIdentity)
> > > + // More specialized overload for complete object types.
> > > + template<typename _Tp,
> > > + typename = __enable_if_t<!__or_<__not_<is_object<_Tp>>,
> > > +
> > > __is_array_unknown_bounds<_Tp>>::value>,
> >
> > Maybe it's because I'm congested and my head feels like it's full of
> > potatoes, but the double negative is confusing for me.
> >
> > Would __and_<is_object<T>, __not_<__is_array_unknown_bounds<T>> work?
> >
> > We could even name that:
> >
> > // An object type which is not an unbounded array.
> > // It might still be an incomplete type, but if this is false_type
> > // then we can be certain it's not a complete object type.
> > template<typename _Tp>
> > using __maybe_complete_object_type = ...;
>
> I like it.
>
> >
> >
> > > + size_t = sizeof(_Tp)>
> > > + constexpr true_type
> > > + __is_complete_or_unbounded(__type_identity<_Tp>)
> > > + { return {}; };
> > > +
> > > + // Less specialized overload for reference and unknown-bound array
> > > types, and
> > > + // incomplete types.
> > > + template<typename _TypeIdentity,
> > > + typename _NestedType = typename _TypeIdentity::type>
> > > + constexpr typename __or_<__not_<is_object<_NestedType>>,
> > > + __is_array_unknown_bounds<_NestedType>>::type
> >
> > Then this would be __not_<__maybe_complete_object_type<_NestedType>>
> > but maybe that's as bad as the double negative above.
>
> The new helper definitely makes things clearer to me. Like so?
Yeah, I find that easier to follow even with a head full of potato ;-)
OK for trunk, thanks.
>
> -- >8 --
>
> Subject: [PATCH] libstdc++: Unnecessary type completion in
> __is_complete_or_unbounded [PR120717]
>
> When checking __is_complete_or_unbounded on a reference to incomplete
> type, we overeagerly try to instantiate/complete the referenced type
> which besides being unnecessary may also produce a -Wsfinae-incomplete
> warning (added in r16-1527) if the referenced type is later defined.
>
> This patch fixes this by effectively restricting the sizeof check to
> object (except unknown-bound array) types. In passing simplify the
> implementation by using is_object instead of is_function/reference/void
> and introducing a __maybe_complete_object_type helper.
>
> PR libstdc++/120717
>
> libstdc++-v3/ChangeLog:
>
> * include/std/type_traits (__maybe_complete_object_type): New
> trait, factored out from ...
> (__is_complete_or_unbounded): ... here. Only check sizeof on a
> __maybe_complete_object_type type. Fix formatting.
> * testsuite/20_util/is_complete_or_unbounded/120717.cc: New test.
> ---
> libstdc++-v3/include/std/type_traits | 39 +++++++++++--------
> .../is_complete_or_unbounded/120717.cc | 20 ++++++++++
> 2 files changed, 42 insertions(+), 17 deletions(-)
> create mode 100644
> libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/120717.cc
>
> diff --git a/libstdc++-v3/include/std/type_traits
> b/libstdc++-v3/include/std/type_traits
> index abff9f880001..055411195f17 100644
> --- a/libstdc++-v3/include/std/type_traits
> +++ b/libstdc++-v3/include/std/type_traits
> @@ -280,11 +280,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> // Forward declarations
> template<typename>
> - struct is_reference;
> - template<typename>
> - struct is_function;
> - template<typename>
> - struct is_void;
> + struct is_object;
> template<typename>
> struct remove_cv;
> template<typename>
> @@ -294,21 +290,30 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> template<typename>
> struct __is_array_unknown_bounds;
>
> + // An object type which is not an unbounded array.
> + // It might still be an incomplete type, but if this is false_type
> + // then we can be certain it's not a complete object type.
> + template<typename _Tp>
> + using __maybe_complete_object_type
> + = __and_<is_object<_Tp>, __not_<__is_array_unknown_bounds<_Tp>>>;
> +
> // Helper functions that return false_type for incomplete classes,
> // incomplete unions and arrays of known bound from those.
>
> - template <typename _Tp, size_t = sizeof(_Tp)>
> - constexpr true_type __is_complete_or_unbounded(__type_identity<_Tp>)
> - { return {}; }
> -
> - template <typename _TypeIdentity,
> - typename _NestedType = typename _TypeIdentity::type>
> - constexpr typename __or_<
> - is_reference<_NestedType>,
> - is_function<_NestedType>,
> - is_void<_NestedType>,
> - __is_array_unknown_bounds<_NestedType>
> - >::type __is_complete_or_unbounded(_TypeIdentity)
> + // More specialized overload for complete object types (returning
> true_type).
> + template<typename _Tp,
> + typename = __enable_if_t<__maybe_complete_object_type<_Tp>::value>,
> + size_t = sizeof(_Tp)>
> + constexpr true_type
> + __is_complete_or_unbounded(__type_identity<_Tp>)
> + { return {}; };
> +
> + // Less specialized overload for reference and unknown-bound array types
> + // (returning true_type), and incomplete types (returning false_type).
> + template<typename _TypeIdentity,
> + typename _NestedType = typename _TypeIdentity::type>
> + constexpr typename
> __not_<__maybe_complete_object_type<_NestedType>>::type
> + __is_complete_or_unbounded(_TypeIdentity)
> { return {}; }
>
> // __remove_cv_t (std::remove_cv_t for C++11).
> diff --git
> a/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/120717.cc
> b/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/120717.cc
> new file mode 100644
> index 000000000000..31fdf8fe9227
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/is_complete_or_unbounded/120717.cc
> @@ -0,0 +1,20 @@
> +// PR libstdc++/120717
> +// { dg-do compile { target c++11 } }
> +// { dg-additional-options "-Wsfinae-incomplete" }
> +
> +#include <type_traits>
> +
> +// Verify __is_complete_or_unbounded doesn't try to instantiate the
> underlying
> +// type of a reference or array of unknown bound.
> +template<class T> struct A { static_assert(false, "do not instantiate"); };
> +static_assert(std::__is_complete_or_unbounded(std::__type_identity<A<int>&>{}),
> "");
> +static_assert(std::__is_complete_or_unbounded(std::__type_identity<A<int>&&>{}),
> "");
> +static_assert(std::__is_complete_or_unbounded(std::__type_identity<A<int>[]>{}),
> "");
> +
> +// Verify __is_complete_or_unbounded doesn't produce -Wsfinae-incomplete
> +// warnings.
> +struct B;
> +static_assert(std::__is_complete_or_unbounded(std::__type_identity<B&>{}),
> "");
> +static_assert(std::__is_complete_or_unbounded(std::__type_identity<B&&>{}),
> "");
> +static_assert(std::__is_complete_or_unbounded(std::__type_identity<B[]>{}),
> "");
> +struct B { }; // { dg-bogus "-Wsfinae-incomplete" }
> --
> 2.50.0.81.gcb3b40381e
>
