[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[ppalka at gcc dot gnu.org]

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

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

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

--- Comment #3 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Paolo, would you happen to be working on this PR still?

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
           Assignee|paolo.carlini at oracle dot com    |unassigned at gcc dot gnu.org

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Not actively. I'm unassigning myself.

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[ppalka at gcc dot gnu.org]

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

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

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

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[ppalka at gcc dot gnu.org]

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

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Here is a rejects-valid testcase caused by the same underlying issue:

$ cat testcase.C
struct A { A *p = this; int n; };

constexpr A
foo()
{
  return {};
}

constexpr A
bar()
{
  A a = foo();
  a.p->n = 5;
  return a;
}

static_assert(bar().n == 5);

$ g++ -std=c++2a testcase.C
testcase.C:17:23: error: non-constant condition for static assertion
   17 | static_assert(bar().n == 5);
      |               ~~~~~~~~^~~~
testcase.C:17:19:   in ‘constexpr’ expansion of ‘bar()’
testcase.C:17:23: error: modification of ‘‘result_decl’ not supported by
dump_expr<expression error>’ is not a constant expression

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[jason at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Patrick Palka from comment #5)
> Here is a rejects-valid testcase caused by the same underlying issue:

Actually, A does not have guaranteed copy elision in function argument passing
or return.  Which is a good thing, since it can't have it under the current
ABI.

http://eel.is/c++draft/class.temporary#3 

"When an object of class type X is passed to or returned from a function, if X
has at least one eligible copy or move constructor ([special]), each such
constructor is trivial, and the destructor of X is either trivial or deleted,
implementations are permitted to create a temporary object to hold the function
parameter or result object."

So I suppose my comment on Paolo's patch was wrong, and this is only a
diagnostic quality issue after all.

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[jason at gcc dot gnu.org]

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

--- Comment #7 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #6)
> So I suppose my comment on Paolo's patch was wrong, and this is only a
> diagnostic quality issue after all.

Ah, there is a constexpr issue to fix: if we add constructors to A, we should
get guaranteed RVO, but still see the same bogus error.

struct A {
  A() = default; A(const A&);
  A *ap = this;
};
constexpr A foo() { return {}; }
static constexpr A a = foo(); // works without 'constexpr'
int main() {
  if (&a != a.ap)
    __builtin_abort();
}

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

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

commit r10-7718-gb256222910cfa4a9b2b477dff8954e51fdc36bb9
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Apr 8 13:14:42 2020 -0400

    c++: Stray RESULT_DECLs in result of constexpr call [PR94034]

    When evaluating the initializer of 'a' in the following example

      struct A {
        A() = default; A(const A&);
        A *p = this;
      };
      constexpr A foo() { return {}; }
      constexpr A a = foo();

    the PLACEHOLDER_EXPR for 'this' in the aggregate initializer returned by
foo
    gets resolved to the RESULT_DECL of foo.  But due to guaranteed RVO, the
'this'
    should really be resolved to '&a'.

    Fixing this properly by immediately resolving 'this' and PLACEHOLDER_EXPRs
to
    the ultimate object under construction would in general mean that we would
no
    longer be able to cache constexpr calls for which RVO possibly applies,
because
    the result of the call may now depend on the ultimate object under
construction.

    So as a mostly correct stopgap solution that retains cachability of RVO'd
    constexpr calls, this patch fixes this issue by rewriting all occurrences
of the
    RESULT_DECL in the result of a constexpr function call with the current
object
    under construction, after the call returns.  This means the 'this' pointer
    during construction of the temporary will still point to the temporary
object
    instead of the ultimate object, but besides that this approach seems
    functionally equivalent to the proper approach.

    gcc/cp/ChangeLog:

            PR c++/94034
            * constexpr.c (replace_result_decl_data): New struct.
            (replace_result_decl_data_r): New function.
            (replace_result_decl): New function.
            (cxx_eval_call_expression): Use it.
            * tree.c (build_aggr_init_expr): Set the location of the
AGGR_INIT_EXPR
            to that of its initializer.

    gcc/testsuite/ChangeLog:

            PR c++/94034
            * g++.dg/cpp0x/constexpr-empty15.C: New test.
            * g++.dg/cpp1y/constexpr-nsdmi6a.C: New test.
            * g++.dg/cpp1y/constexpr-nsdmi6b.C: New test.
            * g++.dg/cpp1y/constexpr-nsdmi7a.C: New test.
            * g++.dg/cpp1y/constexpr-nsdmi7b.C: New test.

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[ppalka at gcc dot gnu.org]

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

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

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

--- Comment #9 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 10.

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:78a6d0e30d7950216dc0c5be5d65d0cbed13924c

commit r11-7263-g78a6d0e30d7950216dc0c5be5d65d0cbed13924c
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue Feb 16 20:49:12 2021 -0500

    c++: Revert EXPR_LOCATION change to build_aggr_init_expr [PR96997]

    My change in r10-7718 to make build_aggr_init_expr set EXPR_LOCATION
    (mimicking build_target_expr) causes the debuginfo regression PR96997.
    Given that this change is mostly independent of the rest of the commit,
    and that the only fallout of reverting it is a less accurate error
    message location in a testcase introduced in the same commit, it seems
    the best way forward is to just revert this part of the commit.

    gcc/cp/ChangeLog:

            PR debug/96997
            PR c++/94034
            * tree.c (build_aggr_init_expr): Revert r10-7718 change.

    gcc/testsuite/ChangeLog:

            PR debug/96997
            PR c++/94034
            * g++.dg/cpp1y/constexpr-nsdmi7b.C:  Adjust expected location of
            "call to non-'constexpr' function" error message.

[Bug c++/94034] [10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

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

commit r10-9372-gd7fa3fa5796a46deb308c8c38b247e290d97f1f6
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Feb 17 09:55:42 2021 -0500

    c++: Revert EXPR_LOCATION change to build_aggr_init_expr [PR96997]

    My change in r10-7718 to make build_aggr_init_expr set EXPR_LOCATION
    (mimicking build_target_expr) causes the debuginfo regression PR96997.
    Given that this change is mostly independent of the rest of the commit,
    and that the only fallout of reverting it is a less accurate error
    message location in a testcase introduced in the same commit, it seems
    the best way forward is to just revert this part of the commit.

    gcc/cp/ChangeLog:

            PR debug/96997
            PR c++/94034
            * tree.c (build_aggr_init_expr): Revert r10-7718 change.

    gcc/testsuite/ChangeLog:

            PR debug/96997
            PR c++/94034
            * g++.dg/cpp1y/constexpr-nsdmi7b.C:  Adjust expected location of
            "call to non-'constexpr' function" error message.

    (cherry picked from commit 78a6d0e30d7950216dc0c5be5d65d0cbed13924c)