[Bug c++/116015] New: ICE in replace_placeholders_r for simple default member initializer

[Bug c++/116015] New: ICE in replace_placeholders_r for simple default member initializer

[pieter.p.dev at outlook dot com]

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

            Bug ID: 116015
           Summary: ICE in replace_placeholders_r for simple default
                    member initializer
           Product: gcc
           Version: 14.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pieter.p.dev at outlook dot com
  Target Milestone: ---

Created attachment 58713
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58713&action=edit
Output from -freport-bug

G++ crashes when using member variables in the default initializer for another
member. Minimal example:

    struct MatrixLayout {
        int rows         = 0;
        int outer_stride = rows;
    };
    struct Matrix {
        Matrix(MatrixLayout) {}
    };
    struct Widget {
        int n = 5;
        Matrix A{{.rows = n}};
    };
    int main() {
        Widget w{};
    }

All versions of GCC between at least 5.1 and 14.1 seem to be affected:
  - 5.1: https://godbolt.org/z/1PKx8djxG
  - 14.1: https://godbolt.org/z/1eeEcYoqd
  - trunk: https://godbolt.org/z/7j7n38ohh
(8d6498f0d772775ad09d7d9e14f491a306e5f853)

Possibly related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100252#c5.

To the best of my knowledge, the source code is valid C++20 (and valid C++11
without the designated initializer), and Clang and MSVC compile the same code
without issues.

Output from GCC 14.1:

    ice.cpp: In function 'int main()':
    ice.cpp:16:14: internal compiler error: in replace_placeholders_r, at
cp/tree.cc:3414
       16 |     Widget w{};
          |              ^
    0x732cae replace_placeholders_r
        cp/tree.cc:3414
    0x11be9e3 walk_tree_1(tree_node**, tree_node* (*)(tree_node**, int*,
void*), void*, hash_set<tree_node*, false, default_hash_traits<tree_node*> >*,
tree_node* (*)(tree_node**, int*, tree_node* (*)(tree_node**, int*, void*),
void*, hash_set<tree_node*, false, default_hash_traits<tree_node*> >*))
        tree.cc:11429
    0x11bef2f walk_tree_1(tree_node**, tree_node* (*)(tree_node**, int*,
void*), void*, hash_set<tree_node*, false, default_hash_traits<tree_node*> >*,
tree_node* (*)(tree_node**, int*, tree_node* (*)(tree_node**, int*, void*),
void*, hash_set<tree_node*, false, default_hash_traits<tree_node*> >*))
        tree.cc:11663
    0x960b0c replace_placeholders_r
        cp/tree.cc:3459
    0x11be9e3 walk_tree_1(tree_node**, tree_node* (*)(tree_node**, int*,
void*), void*, hash_set<tree_node*, false, default_hash_traits<tree_node*> >*,
tree_node* (*)(tree_node**, int*, tree_node* (*)(tree_node**, int*, void*),
void*, hash_set<tree_node*, false, default_hash_traits<tree_node*> >*))
        tree.cc:11429
    0x95d985 replace_placeholders(tree_node*, tree_node*, bool*)
        cp/tree.cc:3497
    0x7e0dde cp_gimplify_init_expr
        cp/cp-gimplify.cc:339
    0x7e0dde cp_gimplify_expr(tree_node**, gimple**, gimple**)
        cp/cp-gimplify.cc:652
    0xc139b8 gimplify_expr(tree_node**, gimple**, gimple**, bool
(*)(tree_node*), int)
        gimplify.cc:17790
    0xc17616 gimplify_stmt(tree_node**, gimple**)
        gimplify.cc:7578
    0xc19632 gimplify_and_add(tree_node*, gimple**)
        gimplify.cc:515
    0xc19632 internal_get_tmp_var
        gimplify.cc:674
    0xc13c7b gimplify_expr(tree_node**, gimple**, gimple**, bool
(*)(tree_node*), int)
        gimplify.cc:18908
    0xc1b564 gimplify_expr
        gimplify.cc:18953
    0xc1b564 gimplify_arg(tree_node**, gimple**, unsigned int, bool)
        gimplify.cc:3779
    0xc1da17 gimplify_call_expr
        gimplify.cc:4064
    0xc15ac2 gimplify_expr(tree_node**, gimple**, gimple**, bool
(*)(tree_node*), int)
        gimplify.cc:17853
    0xc17616 gimplify_stmt(tree_node**, gimple**)
        gimplify.cc:7578
    0xc15607 gimplify_cleanup_point_expr
        gimplify.cc:7316
    0xc15607 gimplify_expr(tree_node**, gimple**, gimple**, bool
(*)(tree_node*), int)
        gimplify.cc:18277

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |mpolacek at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |ice-on-valid-code
   Last reconfirmed|                            |2024-07-20

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Started with r5-4459-g3e605b20a0c804:

commit 3e605b20a0c804d57d4be0f4f2bbab8b4e42fce6
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Oct 27 13:42:12 2014 -0400

    Implement N3653 (Member initializers and aggregates) and fix references to
'this' in constexpr constructors.

            Implement N3653 (Member initializers and aggregates) and fix
            references to 'this' in constexpr constructors.

also see r12-6028-ga37e8ce3b66325.

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[mpolacek at gcc dot gnu.org]

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

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
(Not a regression; previously we rejected the code.)

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[pieter.p.dev at outlook dot com]

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

--- Comment #3 from Pieter P <pieter.p.dev at outlook dot com> ---
A temporary workaround is to wrap the default value in an immediately invoked
lambda:

    struct Widget {
        int n = 5;
        Matrix A = [this] { return Matrix{{.rows = n}}; }();
    };

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ppalka at gcc dot gnu.org
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=100252

--- Comment #4 from Patrick Palka <ppalka at gcc dot gnu.org> ---
This does look very similar to PR100252#c5

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[mpolacek at gcc dot gnu.org]

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

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Yeah.  (I would like to take a look soon but I'm not going to assign it to
myself until I have a patch/know what exactly is going on.)

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[mpolacek at gcc dot gnu.org]

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

--- Comment #6 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
  Matrix A1{{n}};

ICEs too, but not

  Matrix A0{{}};
  Matrix A2{{n, n}};

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[mpolacek at gcc dot gnu.org]

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

--- Comment #7 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
As in bug 100252, we have:

D.2872 = {.rows=(&<PLACEHOLDER_EXPR struct Widget>)->n,
.outer_stride=(&<PLACEHOLDER_EXPR struct MatrixLayout>)->rows}

that is, two PLACEHOLDER_EXPRs for different types on the same level in one {
}.

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #8 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Testing a patch.

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

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

commit r15-2920-gd91b6c93f98cac71f5588d73191d08ad788e600c
Author: Marek Polacek <polacek@redhat.com>
Date:   Fri Aug 9 16:14:18 2024 -0400

    c++: ICE with NSDMIs and fn arguments [PR116015]

    The problem in this PR is that we ended up with

      {.rows=(&<PLACEHOLDER_EXPR struct Widget>)->n,
       .outer_stride=(&<PLACEHOLDER_EXPR struct MatrixLayout>)->rows}

    that is, two PLACEHOLDER_EXPRs for different types on the same level
    in one { }.  That should not happen; we may, for instance, neglect to
    replace a PLACEHOLDER_EXPR due to CONSTRUCTOR_PLACEHOLDER_BOUNDARY on
    the constructor.

    The same problem happened in PR100252, which I fixed by introducing
    replace_placeholders_for_class_temp_r.  That didn't work here, though,
    because r_p_for_c_t_r only works for non-eliding TARGET_EXPRs: replacing
    a PLACEHOLDER_EXPR with a temporary that is going to be elided will
    result in a crash in gimplify_var_or_parm_decl when it encounters such
    a loose decl.

    But leaving the PLACEHOLDER_EXPRs in is also bad because then we end
    up with this PR.

    TARGET_EXPRs for function arguments are elided in gimplify_arg.  The
    argument will get a real temporary only in get_formal_tmp_var.  One
    idea was to use the temporary that is going to be elided anyway, and
    then replace_decl it with the real object once we get it.  But that
    didn't work out: one problem is that we elide the TARGET_EXPR for an
    argument before we create the real temporary for the argument, and
    when we get it, the context that this was a TARGET_EXPR for an argument
    has been lost.  We're also in the middle end territory now, even though
    this is a C++-specific problem.

    A solution is to simply stop eliding TARGET_EXPRs whose initializer is
    a CONSTRUCTOR.  Such copies can't be (at the moment) elided anyway.  But
    not eliding all TARGET_EXPRs would be a pessimization.

            PR c++/116015

    gcc/cp/ChangeLog:

            * call.cc (convert_for_arg_passing): Don't set_target_expr_eliding
            when the TARGET_EXPR initializer is a CONSTRUCTOR.

    gcc/ChangeLog:

            * gimplify.cc (gimplify_arg): Do not strip a TARGET_EXPR whose
            initializer is a CONSTRUCTOR.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp1y/nsdmi-aggr23.C: New test.

[Bug c++/116015] ICE in replace_placeholders_r for simple default member initializer

[mpolacek at gcc dot gnu.org]

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

--- Comment #10 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Fixed on trunk so far.  I think it makes sense to fix this in 14 too.