[Bug c++/106838] New: Built-in traits have wrong preconditions

[Bug c++/106838] New: Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

            Bug ID: 106838
           Summary: Built-in traits have wrong preconditions
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
  Target Milestone: ---

According to https://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html the
precondition for __has_virtual_destructor is:

"Requires: type shall be a complete type, (possibly cv-qualified) void, or an
array of unknown bound."

This seems to be what the implementation actually requires:

union Inc;
static_assert( !__has_virtual_destructor(Inc));
struct Inc2;
static_assert( !__has_virtual_destructor(Inc2[1]) );


inc.C:2:45: error: invalid use of incomplete type 'union Inc'
    2 | static_assert( !__has_virtual_destructor(Inc));
      |                                             ^
inc.C:1:7: note: forward declaration of 'union Inc'
    1 | union Inc;
      |       ^~~
inc.C:4:49: error: invalid use of incomplete type 'struct Inc2'
    4 | static_assert( !__has_virtual_destructor(Inc2[1]) );
      |                                                 ^
inc.C:3:8: note: forward declaration of 'struct Inc2'
    3 | struct Inc2;
      |        ^~~~


However, the precondition for the std::has_virtual_destructor trait is
different:

"If T is a non-union class type, T shall be a complete type."

So std::has_virtual_destructor_v<Inc> and
std::has_virtual_destructor_v<Inc2[1]> should both be false. But because the
library trait just uses __has_virtual_destructor, those are ill-formed.

Should the built-in (and its documentation) be adjusted to match the library's
requirements?

Clang gives the same errors as GCC. EDG rejects the incomplete union, but
allows the array of incomplete type. This suggests that the library trait needs
to avoid using the built-in for those types, even if GCC's built-in is changed
to support it.

[Bug c++/106838] Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
According to https://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html the
precondition for __is_trivial is:

"Requires: type shall be a complete type, (possibly cv-qualified) void, or an
array of unknown bound."

That implies the following should be valid:

struct Inc;
bool b = __is_trivial(Inc[]);

However, the implementation seems to match the requirement for the
std::is_trivial trait, which is:
"remove_all_extents_t<T> shall be a complete type or cv void."

This seems to be just a documentation bug.

[Bug c++/106838] Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The documented preconditions for __is_literal_type, __is_pod,
__is_standard_layout, and __is_trivial say:

Requires: type shall be a complete type, (possibly cv-qualified) void, or an
array of unknown bound.

But the corresponding library trait says:

"remove_all_extents_t<T> shall be a complete type or cv void."

I think r13-25 was right to allow arrays of unknown bound, but there should
also be a check that the array element is complete.


Looks like __is_aggregate has the same problem, but isn't documented in the
manual.

[Bug c++/106838] Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We have three main kinds of trait preconditions.


A. "T shall be a complete type, (possibly cv-qualified) void, or an array of
unknown bound."

This only allows T[1] is T is complete, but always allows T[], even if
incomplete.

The rationale here is that we can always tell if T[] is constructible,
destructible, assignable etc (the answer is always no), but to tell whether
T[1] is constructible we need to know about T.


B. "remove_all_extents_t<T> shall be a complete type or cv void."

This only allows T[] and T[1] when T is complete.

The rationale is that we need to know properties of the array element, e.g. to
tell whether T[1] is an aggregate, we need to know if T is an aggregate.


C. "If T is a non-union class type, T shall be a complete type."

This always allows T[] and T[1], even if T is incomplete, and allows incomplete
T is T is a union type.

The rationale is that these traits are always false for array types, e.g. T[]
is not a polymorphic type, even if T is polymorphic.


Currently it seems that check_trait_type only supports preconditions of kind A,
and since r13-25 it allows all arrays, when it should only allow unbounded
arrays. So e.g. it doesn't reject __is_constructible(Incomplete[1]) even though
it can't know the answer.


I think check_trait_type could be adjusted to handle kinds A and B, by adding a
bool parameter, but maybe kind C should be checked by a separate function, as
it is quite different.

[Bug c++/106838] Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
All traits of kind C currently reject T[] and T[1], but should accept them.
That applies to:

__is_empty
__is_polymorphic
__is_abstract
__has_virtual_destructor

These should accept Incomplete[] and Incomplete[1] and just return false.

It's possible that the non-standard "__has_xxx" traits should also be of kind
C, e.g. __has_trivial_assign(Incomplete[]) could be false, because arrays
aren't assignable. But I don't really care about those ones, because the
library doesn't use them (only __has_trivial_destructor, which is used for
std::is_trivially_destructible, but the library can ensure we never use it with
arrays, if necessary).

[Bug c++/106838] Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #4)
> All traits of kind C currently reject T[] and T[1], but should accept them.

Gah, they do in gcc-12, but not trunk, sorry.

But they do reject incomplete unions, which should be accepted.

And traits of kind A incorrectly allow Incomplete[1].

And traits of kind B incorrectly allow Incomplete[].


Here's a testcase:

struct I;

// A.
// "T shall be a complete type, (possibly cv-qualified) void,"
// "or an array of unknown bound."

void kindA()
{
  // No incomplete object types.
  (void) __is_constructible(I); // { dg-error "incomplete type" "FAILS" }
  (void) __is_constructible(I[1]); // { dg-error "incomplete type" "FAILS" }
  // Except arrays of unknown bound.
  (void) __is_constructible(I[]);
}

// B.
// "remove_all_extents_t<T> shall be a complete type or cv void."

void kindB()
{
  // No incomplete types, including arrays.
  (void) __is_trivial(I); // { dg-error "incomplete type" }
  (void) __is_trivial(I[1]); // { dg-error "incomplete type" }
  (void) __is_trivial(I[]); // { dg-error "incomplete type" "FAILS" }
}

union U;

// C.
// "If T is a non-union class type, T shall be a complete type."

void kindC()
{
  // No incomplete non-union class types.
  (void) __is_empty(I); // { dg-error "incomplete type" }
  // But arrays of incomplete type are OK.
  (void) __is_empty(I[1]); // { dg-bogus "incomplete type" "FAILS" }
  (void) __is_empty(I[]);
  // And incomplete unions are OK.
  (void) __is_empty(U); // { dg-bogus "incomplete type" "FAILS" }
  (void) __is_empty(U[1]); // { dg-bogus "incomplete type" "FAILS" }
  (void) __is_empty(U[]);
}

The dg-error lines marked FAIL are incorrectly accepted. The dg-bogus lines are
incorrectly rejected.

[Bug c++/106838] Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #5)
>   (void) __is_trivial(I[]); // { dg-error "incomplete type" "FAILS" }

This gives an error in gcc-12, so is a regression.

But gcc-12 also gives additional bogus errors, so overall trunk is closer to
being right. But still not right.

[Bug c++/106838] Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Oh, and actually __is_final is a fourth kind:

// D.
// "If T is a class type, T shall be a complete type."

So the same as C, but doesn't allow incomplete unions.

[Bug c++/106838] Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
   Last reconfirmed|                            |2022-09-06

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Created attachment 53543
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53543&action=edit
c++: Fix type completeness checks for type traits [PR106838]

Untested patch

[Bug c++/106838] Built-in traits have wrong preconditions

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:30c811f2bac73e63e0b461ba7ed3805b77898798

commit r13-2550-g30c811f2bac73e63e0b461ba7ed3805b77898798
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Sep 6 13:21:09 2022 +0100

    c++: Fix type completeness checks for type traits [PR106838]

    The check_trait_type function is used for a number of different type
    traits that have different requirements on their arguments. For example,
    __is_constructible allows arrays of unknown bound even if the array
    element is an incomplete type, but __is_aggregate does not, it always
    requires the array element type to be complete. Other traits have
    different requirements again, e.g. __is_empty allows incomplete unions,
    and arrays (of known or unknown bound) of incomplete types.

    This alters the check_trait_type function to take an additional KIND
    parameter which indicates which set of type trait requirements to check.

    As noted in a comment, the requirements for __is_aggregate deviate from
    the ones for std::is_aggregate in the standard. It's not necessary for
    the elements of an array to be complete types, because arrays are always
    aggregates.

    The type_has_virtual_destructor change is needed to avoid an ICE.
    Previously it could never be called for incomplete union types as they
    were (incorrectly) rejected by check_trait_type.

    This change causes some additional diagnostics in some libstdc++ tests,
    where the front end was not previously complaining about invalid types
    that the library assertions diagnosed. We should consider removing the
    library assertions from traits where the front end implements the
    correct checks now.

            PR c++/106838

    gcc/cp/ChangeLog:

            * class.cc (type_has_virtual_destructor): Return false for
            union types.
            * semantics.cc (check_trait_type): Add KIND parameter to support
            different sets of requirements.
            (finish_trait_expr): Pass KIND argument for relevant traits.

    gcc/ChangeLog:

            * doc/extend.texi (Type Traits): Fix requirements. Document
            __is_aggregate and __is_final.

    gcc/testsuite/ChangeLog:

            * g++.dg/ext/array4.C: Fix invalid use of __is_constructible.
            * g++.dg/ext/unary_trait_incomplete.C: Fix tests for traits with
            different requirements.

    libstdc++-v3/ChangeLog:

            * testsuite/20_util/is_complete_or_unbounded/memoization_neg.cc:
            Prune additional errors from front-end.
            * testsuite/20_util/is_move_constructible/incomplete_neg.cc:
            Likewise.
            * testsuite/20_util/is_nothrow_swappable/incomplete_neg.cc:
            Likewise.
            * testsuite/20_util/is_nothrow_swappable_with/incomplete_neg.cc:
            Likewise.
            * testsuite/20_util/is_swappable_with/incomplete_neg.cc:
            Likewise.

[Bug c++/106838] Built-in traits have wrong preconditions

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106838

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
   Target Milestone|---                         |13.0
         Resolution|---                         |FIXED

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk.