On Wed, 4 Feb 2026, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase is miscompiled since r0-69852-g4038c495f (at least
> if one can somehow arrange in C++98 to have AGGR_INIT_EXPR or any other
> FE specific trees nested in constructor elts for CONSTRUCTOR nested inside
> of default argument, if not, then since C++11 support that allows that has
> been implemented).
> The problem is that we unfortunately store default arguments in TREE_PURPOSE
> of TYPE_ARG_TYPES nodes of the FUNCTION/METHOD_TYPE and those are shared,
> with type_hash_canon used to unify them.  The default arguments aren't
> considered in type_hash_canon_hash at all, but the equality hook considers
> them in type_list_equal by calling simple_cst_equal on those default
> arguments.  That function is a tri-state, returns 0 for surely unequal,
> 1 for equal and -1 for "I don't know", usually because there are FE trees
> etc. (though, admittedly it is unclear why such distinction is done, as
> e.g. for VAR_DECLs etc. it sees in there it returns 0 rather than -1).
> Anyway, the r0-69852-g4038c495f change changed CONSTRUCTOR_ELTS from
> I think a tree list of elements to a vector and changed the simple_cst_equal
> implementation of CONSTRUCTOR from just recursing on CONSTRUCTOR_ELTS (which
> I think would just return -1 because I don't see simple_cst_equal having
> TREE_LIST nor TREE_VEC handling) to something that thinks simple_cst_equal
> returns a bool rather than tri-state, plus has a comment whether it should
> handle indexes too.
> So, in particular on the testcase which has in default arguments with
> CONSTRUCTOR inside it with AGGR_INIT_EXPR as the single element (but
> could be as well in multiple elements), the recursive call returns -1
> for "I don't know" on the element and the function considers it the same
> as if it returned 1, they are equal.
> 
> The following patch fixes it by calling the recursive non tail-recursive
> simple_cst_equal calls like everywhere else in the function, by returning
> what it returned if it returned <= 0 and otherwise continuing.
> Plus given the comment I've also implemented checking the index.
> The special case for FIELD_DECL is so that if both indexes are FIELD_DECLs
> and they are different, we don't return -1 but 0.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> 2026-02-04  Jakub Jelinek  <[email protected]>
> 
>       PR c++/123818
>       * tree.cc (simple_cst_equal) <case CONSTRUCTOR>: Return -1 if some
>       recursive call returns -1.  Also compare indexes.
> 
>       * g++.dg/cpp0x/pr123818.C: New test.
> 
> --- gcc/tree.cc.jj    2026-01-17 14:37:22.171617643 +0100
> +++ gcc/tree.cc       2026-02-03 16:16:02.637247532 +0100
> @@ -6826,13 +6826,27 @@ simple_cst_equal (const_tree t1, const_t
>       vec<constructor_elt, va_gc> *v2 = CONSTRUCTOR_ELTS (t2);
>  
>       if (vec_safe_length (v1) != vec_safe_length (v2))
> -       return false;
> +       return 0;
>  
>          for (idx = 0; idx < vec_safe_length (v1); ++idx)
> -       /* ??? Should we handle also fields here? */
> -       if (!simple_cst_equal ((*v1)[idx].value, (*v2)[idx].value))
> -         return false;
> -     return true;
> +       {
> +         if ((*v1)[idx].index
> +             && TREE_CODE ((*v1)[idx].index) == FIELD_DECL)
> +           {
> +             if ((*v1)[idx].index != (*v2)[idx].index)
> +               return 0;
> +           }
> +         else
> +           {
> +             cmp = simple_cst_equal ((*v1)[idx].index, (*v2)[idx].index);
> +             if (cmp <= 0)
> +               return cmp;
> +           }
> +         cmp = simple_cst_equal ((*v1)[idx].value, (*v2)[idx].value);
> +         if (cmp <= 0)
> +           return cmp;
> +       }
> +     return 1;
>        }
>  
>      case SAVE_EXPR:
> --- gcc/testsuite/g++.dg/cpp0x/pr123818.C.jj  2026-02-03 16:53:44.557317464 
> +0100
> +++ gcc/testsuite/g++.dg/cpp0x/pr123818.C     2026-02-03 16:54:25.224618054 
> +0100
> @@ -0,0 +1,24 @@
> +// PR c++/123818
> +// { dg-do run { target c++11 } }
> +
> +struct A { A (int x) : a (x) {} int a; };
> +struct B { A b; };
> +
> +int
> +foo (B x = B { 42 })
> +{
> +    return x.b.a;
> +}
> +
> +int
> +bar (B x = B { 43 })
> +{
> +    return x.b.a;
> +}
> +
> +int
> +main ()
> +{
> +  if (foo () != bar () - 1)
> +    __builtin_abort ();
> +}
> 
>       Jakub
> 
> 

-- 
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to