[Bug c++/109277] New: [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[Bug c++/109277] New: [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[marxin at gcc dot gnu.org]

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

            Bug ID: 109277
           Summary: [13 Regression] type_traits:1417:30: error: invalid
                    use of incomplete type ‘class v8::internal::WasmArray’
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: rejects-valid
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: redi at gcc dot gnu.org
  Target Milestone: ---

Reduced from chromium, gcc-12 accepts the code:

GCC 13 pre-processed source code:
https://gist.githubusercontent.com/marxin/377cbc8221f7c36e87aebe48a55b52ba/raw/b80443940faf20f6f5db124fb6a56d9bee994fee/promise13.ii

$ g++ -std=gnu++20 promise13.ii -c -w
In file included from /usr/include/math.h:398,
                 from /usr/include/c++/13/cmath:47,
                 from ../v8/src/utils/utils.h:12,
                 from ../v8/src/zone/zone.h:15,
                 from ../v8/src/handles/handles.h:14,
                 from ../v8/src/ast/ast-value-factory.h:36,
                 from ../v8/src/ast/ast.h:10,
                 from
gen/v8/torque-generated/src/builtins/promise-all-element-closure-tq-csa.cc:1:
/usr/include/bits/mathcalls-helper-functions.h: In static member function
‘static int __iseqsig_type<_Float128>::__call(_Float128, _Float128)’:
/usr/include/bits/mathcalls-helper-functions.h:41:36: note:   initializing
argument 1 of ‘int __iseqsigl(long double, long double)’
   41 | __MATHDECL_ALIAS (int, __iseqsig,, (_Mdouble_ __x, _Mdouble_ __y),
iseqsig);
      |                        ~~~~~~~~~~~~^~~
/usr/include/bits/mathcalls-helper-functions.h:41:53: note:   initializing
argument 2 of ‘int __iseqsigl(long double, long double)’
   41 | __MATHDECL_ALIAS (int, __iseqsig,, (_Mdouble_ __x, _Mdouble_ __y),
iseqsig);
      |                                         ~~~~~~~~~~~~^~~
In file included from /usr/include/c++/13/bits/move.h:57,
                 from /usr/include/c++/13/bits/new_allocator.h:36,
                 from
/usr/include/c++/13/aarch64-suse-linux/bits/c++allocator.h:33,
                 from /usr/include/c++/13/bits/allocator.h:46,
                 from /usr/include/c++/13/memory:65,
                 from ../v8/src/ast/ast.h:8:
/usr/include/c++/13/type_traits: In instantiation of ‘struct
std::is_convertible<v8::internal::WasmArray, v8::internal::Object>’:
../v8/src/codegen/tnode.h:273:72:   required from ‘const bool
v8::internal::is_subtype<v8::internal::WasmArray,
v8::internal::WasmArray>::value’
../v8/src/codegen/tnode.h:361:75:   required by substitution of ‘template<class
U, typename std::enable_if<v8::internal::is_subtype<U,
v8::internal::WasmArray>::value, int>::type <anonymous> >
v8::internal::TNode<v8::internal::WasmArray>::TNode(const
v8::internal::TNode<T>&) [with U = v8::internal::WasmArray; typename
std::enable_if<v8::internal::is_subtype<U, v8::internal::WasmArray>::value,
int>::type <anonymous> = <missing>]’
/usr/include/c++/13/tuple:188:12:   required from ‘struct std::_Head_base<0,
v8::internal::TNode<v8::internal::WasmArray>, false>’
/usr/include/c++/13/tuple:259:12:   required from ‘struct std::_Tuple_impl<0,
v8::internal::TNode<v8::internal::WasmArray>,
v8::internal::TNode<v8::internal::IntPtrT>,
v8::internal::TNode<v8::internal::IntPtrT> >’
/usr/include/c++/13/tuple:746:11:   required from ‘class
std::tuple<v8::internal::TNode<v8::internal::WasmArray>,
v8::internal::TNode<v8::internal::IntPtrT>,
v8::internal::TNode<v8::internal::IntPtrT> >’
gen/v8/torque-generated/csa-types.h:487:80:   required from here
/usr/include/c++/13/type_traits:1417:30: error: invalid use of incomplete type
‘class v8::internal::WasmArray’
 1417 |     : public __bool_constant<__is_convertible(_From, _To)>
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../v8/src/heap/factory-base.h:15,
                 from ../v8/src/heap/factory.h:18,
                 from ../v8/src/ast/ast.h:18:
gen/v8/torque-generated/class-forward-declarations.h:291:7: note: forward
declaration of ‘class v8::internal::WasmArray’
In file included from ../v8/src/codegen/interface-descriptors.h:13,
                 from ../v8/src/codegen/callable.h:8,
                 from ../v8/src/codegen/code-factory.h:8,
                 from ../v8/src/codegen/code-stub-assembler.h:12,
                 from ../v8/src/builtins/builtins-array-gen.h:8,
                 from
gen/v8/torque-generated/src/builtins/promise-all-element-closure-tq-csa.cc:2:
../v8/src/codegen/tnode.h: In instantiation of ‘const bool
v8::internal::is_subtype<v8::internal::WasmArray,
v8::internal::WasmArray>::value’:
../v8/src/codegen/tnode.h:361:75:   required by substitution of ‘template<class
U, typename std::enable_if<v8::internal::is_subtype<U,
v8::internal::WasmArray>::value, int>::type <anonymous> >
v8::internal::TNode<v8::internal::WasmArray>::TNode(const
v8::internal::TNode<T>&) [with U = v8::internal::WasmArray; typename
std::enable_if<v8::internal::is_subtype<U, v8::internal::WasmArray>::value,
int>::type <anonymous> = <missing>]’
/usr/include/c++/13/tuple:188:12:   required from ‘struct std::_Head_base<0,
v8::internal::TNode<v8::internal::WasmArray>, false>’
/usr/include/c++/13/tuple:259:12:   required from ‘struct std::_Tuple_impl<0,
v8::internal::TNode<v8::internal::WasmArray>,
v8::internal::TNode<v8::internal::IntPtrT>,
v8::internal::TNode<v8::internal::IntPtrT> >’
/usr/include/c++/13/tuple:746:11:   required from ‘class
std::tuple<v8::internal::TNode<v8::internal::WasmArray>,
v8::internal::TNode<v8::internal::IntPtrT>,
v8::internal::TNode<v8::internal::IntPtrT> >’
gen/v8/torque-generated/csa-types.h:487:80:   required from here
../v8/src/codegen/tnode.h:273:72: error: ‘value’ is not a member of
‘std::is_convertible<v8::internal::WasmArray, v8::internal::Object>’

GCC 12 pre-processed source code:
https://gist.githubusercontent.com/marxin/cca0440a46638a1afc508c908f28e19b/raw/9d2ee4e7e466e55b961f693a769483cf85ba4126/promise12.ii

$ g++-12 -std=gnu++20 promise12.ii -c -w

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-03-24
             Status|UNCONFIRMED                 |NEW
                 CC|                            |jason at gcc dot gnu.org

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |UNCONFIRMED
   Target Milestone|---                         |13.0
     Ever confirmed|1                           |0

[Bug libstdc++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |libstdc++

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The GCC 12 preprocessed source works (with a minor change of
s/__remove_cv/__remove_cv1/ ) so this is definitely due to a libstdc++ change.

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|https://gcc.gnu.org/bugzill |https://gcc.gnu.org/bugzill
                   |a/show_bug.cgi?id=96592     |a/show_bug.cgi?id=107049
          Component|libstdc++                   |c++

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The difference is is_convertible definition.
12:
  template<typename _From, typename _To>
    struct is_convertible
    : public __is_convertible_helper<_From, _To>::type
    { };


13:
  template<typename _From, typename _To>
    struct is_convertible
    : public __bool_constant<__is_convertible(_From, _To)>
    { };

So it might be a front-end issue after all.

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Though the code might be undefined ...

20.15.6/5 says:
```
The predicate condition for a template specialization is_convertible<From, To>
shall be satisfied if and
only if the return expression in the following code would be well-formed,
including any implicit conversions
to the return type of the function:
To test() {
return declval<From>();
}
[Note: This requirement gives well-defined results for reference types, void
types, array types, and function
types. — end note] Access checking is performed in a context unrelated to To
and From. Only the validity
of the immediate context of the expression of the return statement (8.7.3)
(including initialization of the
returned object or reference) is considered. [Note: The initialization can
result in side effects such as
the instantiation of class template specializations and function template
specializations, the generation of
implicitly-defined functions, and so on. Such side effects are not in the
“immediate context” and can result in
the program being ill-formed. — end note]

```
I am not 100% sure but I suspect this is just ill-formed code which was not
rejected before but is now using the builtin rather than the template form ...

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> The difference is is_convertible definition.
> 12:
>   template<typename _From, typename _To>
>     struct is_convertible
>     : public __is_convertible_helper<_From, _To>::type
>     { };
> 
> 
> 13:
>   template<typename _From, typename _To>
>     struct is_convertible
>     : public __bool_constant<__is_convertible(_From, _To)>
>     { };
> 
> So it might be a front-end issue after all.

and yes that is definitely the difference here. If I add back the
__is_convertible_helper and use that for is_convertible to the GCC 13
preprocessed source, the code then compiles.

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Reduced testcase that shows the compiling difference between GCC 12 and 13:
```
#include <type_traits>
struct a;
struct b{};
bool c = std::is_convertible<a,b>::value;
```

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I know cppreference is not the standard but usually it has a good summary.

From: https://en.cppreference.com/w/cpp/types/is_convertible :
```
From and To shall each be a complete type, (possibly cv-qualified) void, or an
array of unknown bound. Otherwise, the behavior is undefined.
```

So someone who understands the C++ standard better than me should be able to
answer if this is in fact illformed code.

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note I filed the note without a warning at PR 109278.

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yes, looks undefined at first glance. We don't have to diagnose bad uses of
type traits (but we don't have to ignore them either, which means both gcc 12
and gcc 13 are correct).

The question is whether the trait instantiation is supposed to happen here, or
if it should be ok to name the incomplete type as long as its not used before
it's complete.

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #9 from Patrick Palka <ppalka at gcc dot gnu.org> ---
AFAICT the trait instantiation is legitimate, and this appears to be UB, here's
a boiled down testcase:

#include <tuple>
#include <type_traits>

struct Object;
struct MaybeObject;
struct WasmArray;

template<class T, class U>
struct is_subtype {
  static const bool value =
      std::is_base_of<U, T>::value || (std::is_same<U, MaybeObject>::value &&
                                       std::is_convertible<T, Object>::value);
};

template<class T>
struct TNode {
  TNode();

  template<class U, typename = std::enable_if_t<is_subtype<U, T>::value>>
  TNode(const TNode<U>&); // #1

  TNode(const TNode&);    // #2
};

std::tuple<TNode<WasmArray>> node;

The instantiation of std::is_convertible<WasmArray, Object> happens when
synthesizing tuple<TNode<WasmArray>> defaulted move constructor, for which we
need to perform overload resolution of TNode's constructors with a TNode&&
argument, and when considering the template candidate #1 we need to instantiate
its default template argument which entails instantiation of is_convertible.

In GCC 12 is_convertible for an incomplete type would silently return false. 
In GCC 13 the new built-in __is_convertible diagnoses this UB situation as a
hard error.  Clang's __is_convertible behaves like GCC 12's is_convertible it
seems.

One fix is to define a move constructor for TNode, which causes GCC's perfect
candidate optimization (r11-7287-g187d0d5871b1fa) to kick in and avoid
considering the template candidate #1.  Another fix is to use
std::conjunction/disjunction in is_subtype so that the condition properly
short-circuits (is_base_of<T, T> is true even for incomplete T).

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|needs-bisection,            |
                   |needs-reduction             |

--- Comment #10 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Thus started with the move to use the built-in __is_convertible in
std::is_convertible, r13-2883-gaf85ad891703db

(In reply to Patrick Palka from comment #9)
> One fix is to define a move constructor for TNode, which causes GCC's
> perfect candidate optimization (r11-7287-g187d0d5871b1fa) to kick in and
> avoid considering the template candidate #1.  Another fix is to use
> std::conjunction/disjunction in is_subtype so that the condition properly
> short-circuits (is_base_of<T, T> is true even for incomplete T).

Note that the first proposed fix would just paper over the UB, whereas the
second fix would eliminate it IIUC.

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
So INVALID?  Is this change in behavior worth documenting in porting_to.html?
(does it make sense to have -fpermissive change the behavior of the builtin?)

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[jason at gcc dot gnu.org]

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

--- Comment #12 from Jason Merrill <jason at gcc dot gnu.org> ---
Created attachment 54806
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54806&action=edit
patch to allow with -fpermissive

Here's an (untested) possible approach to that.  I'm about to be away for a
week, so I won't be following up in that time.

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2023-03-24 00:00:00         |2023-04-13
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:f32f7881fb0db085479525b5a23db5dabd990c3b

commit r13-7172-gf32f7881fb0db085479525b5a23db5dabd990c3b
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Apr 3 23:20:13 2023 -0400

    c++: make trait of incomplete type a permerror [PR109277]

    An incomplete type argument to several traits is specified to be undefined
    behavior in the library; since it's a compile-time property, we diagnose
    it.  But apparently some code was relying on the previous behavior of not
    diagnosing.  So let's make it a permerror.

    The assert in cxx_incomplete_type_diagnostic didn't like that, and I don't
    see the point of having the assert, so let's just remove it.

            PR c++/109277

    gcc/cp/ChangeLog:

            * semantics.cc (check_trait_type): Handle incomplete type directly.
            * typeck2.cc (cxx_incomplete_type_diagnostic): Remove assert.

    gcc/testsuite/ChangeLog:

            * g++.dg/ext/is_convertible5.C: New test.

[Bug c++/109277] [13 Regression] type_traits:1417:30: error: invalid use of incomplete type ‘class v8::internal::WasmArray’

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #14 from Jason Merrill <jason at gcc dot gnu.org> ---
Chromium can now use -fpermissive to make this code compile.  But better to fix
the code, of course.