On Sat, Jan 15, 2022 at 09:24:05AM -0500, Patrick Palka wrote: > On Fri, 14 Jan 2022, Marek Polacek via Gcc-patches wrote: > > > This is a "canonical types differ for identical types" ICE, which started > > with r11-4682. It's a bit tricky to explain. Consider: > > > > template <typename T> struct S { > > S<T> bar() noexcept(T::value); // #1 > > S<T> foo() noexcept(T::value); // #2 > > }; > > > > template <typename T> S<T> S<T>::foo() noexcept(T::value) {} // #3 > > > > We ICE because #3 and #2 have the same type, but their canonical types > > differ: TYPE_CANONICAL (#3) == #2 but TYPE_CANONICAL (#2) == #1. > > > > The member functions #1 and #2 have the same type. However, since their > > noexcept-specifier is deferred, when parsing them, we create a variant for > > both of them, because DEFERRED_PARSE cannot be compared. In other words, > > build_cp_fntype_variant's > > > > tree v = TYPE_MAIN_VARIANT (type); > > for (; v; v = TYPE_NEXT_VARIANT (v)) > > if (cp_check_qualified_type (v, type, type_quals, rqual, raises, late)) > > return v; > > > > will *not* find an existing variant when creating a method_type for #2, so > > we > > have to create a new one. > > > > But then we perform delayed parsing and call > > fixup_deferred_exception_variants > > for #1 and #2. f_d_e_v will replace TYPE_RAISES_EXCEPTIONS with the newly > > parsed noexcept-specifier. It also sets TYPE_CANONICAL (#2) to #1. Both > > noexcepts turned out to be the same, so now we have two equivalent variants > > in > > the list! I.e., > > > > +-----------------+ +-----------------+ +-----------------+ > > | main | | #2 | | #1 | > > | S S::<T379>(S*) |----->| S S::<T37c>(S*) |----->| S S::<T37a>(S*) > > |----->NULL > > | - | | noex(T::value) | | noex(T::value) | > > +-----------------+ +-----------------+ +-----------------+ > > > > Then we get to #3. As for #1 and #2, grokdeclarator calls build_memfn_type, > > which ends up calling build_cp_fntype_variant, which will use the loop > > above to look for an existing variant. The first one that matches > > cp_check_qualified_type will be used, so we use #2 rather than #1, and the > > TYPE_CANONICAL mismatch follows. Hopefully that makes sense. > > > > As for the fix, I didn't think I could rewrite the method_type #2 with #1 > > because the type may have escaped via decltype. So my approach is to > > elide #2 from the list, so when looking for a matching variant, we always > > find #1 (#2 remains live though, which admittedly sounds sort of dodgy). > > I wonder about instead making build_cp_fntype_variant set the TYPE_CANONICAL > for > #3 to TYPE_CANONICAL(#2) (i.e. #1) instead of to #2? Something like: > > -- >8 -- > > gcc/cp/tree.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c > index 7f7de86b4e8..b89135fa121 100644 > --- a/gcc/cp/tree.c > +++ b/gcc/cp/tree.c > @@ -2779,8 +2779,9 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier > rqual, > else if (TYPE_CANONICAL (type) != type || cr != raises || late) > /* Build the underlying canonical type, since it is different > from TYPE. */ > - TYPE_CANONICAL (v) = build_cp_fntype_variant (TYPE_CANONICAL (type), > - rqual, cr, false); > + TYPE_CANONICAL (v) > + = TYPE_CANONICAL (build_cp_fntype_variant (TYPE_CANONICAL (type), > + rqual, cr, false)); > else > /* T is its own canonical type. */ > TYPE_CANONICAL (v) = v;
Thanks for looking. I can dig that (and verified it works), but it strikes me more as a workaround for the duplicity problem. I also don't see TYPE_CANONICAL (...) = TYPE_CANONICAL (build_cp_fntype_variant (...)) anywhere in the codebase, if that means anything. Marek