From: Patrick Palka <ppalka@redhat.com>
To: Nathaniel Shead <nathanieloshead@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH v2 2/3] c++: Improve constexpr error for dangling local variables
Date: Fri, 23 Jun 2023 12:57:59 -0400 (EDT) [thread overview]
Message-ID: <c3a608de-e9c5-05c7-ce92-2632b89f7c49@idea> (raw)
In-Reply-To: <20230329023258.13487-3-nathanieloshead@gmail.com>
On Wed, 29 Mar 2023, Nathaniel Shead via Gcc-patches wrote:
> Currently, when typeck discovers that a return statement will refer to a
> local variable it rewrites to return a null pointer. This causes the
> error messages for using the return value in a constant expression to be
> unhelpful, especially for reference return values.
>
> This patch removes this "optimisation". Relying on this raises a warning
> by default and causes UB anyway, so there should be no issue in doing
> so. We also suppress additional warnings from later passes that detect
> this as a dangling pointer, since we've already indicated this anyway.
LGTM. It seems the original motivation for returning a null pointer
here was to avoid issuing duplicate warnings
(https://gcc.gnu.org/legacy-ml/gcc-patches/2014-04/msg00269.html)
which your patch addresses.
>
> gcc/cp/ChangeLog:
>
> * semantics.cc (finish_return_stmt): Suppress dangling pointer
> reporting on return statement if already reported.
> * typeck.cc (check_return_expr): Don't set return expression to
> zero for dangling addresses.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp1y/constexpr-lifetime5.C: Test reported message is
> correct.
> * g++.dg/warn/Wreturn-local-addr-6.C: Remove check for return
> value optimisation.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
> gcc/cp/semantics.cc | 5 ++++-
> gcc/cp/typeck.cc | 5 +++--
> gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C | 4 ++--
> gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C | 3 ---
> 4 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 87c2e8a7111..14b4b7f4ce1 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -1246,7 +1246,10 @@ finish_return_stmt (tree expr)
>
> r = build_stmt (input_location, RETURN_EXPR, expr);
> if (no_warning)
> - suppress_warning (r, OPT_Wreturn_type);
> + {
> + suppress_warning (r, OPT_Wreturn_type);
> + suppress_warning (r, OPT_Wdangling_pointer_);
> + }
> r = maybe_cleanup_point_expr_void (r);
> r = add_stmt (r);
>
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index afb956087ce..a7d642e2029 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -11235,8 +11235,9 @@ check_return_expr (tree retval, bool *no_warning)
> else if (!processing_template_decl
> && maybe_warn_about_returning_address_of_local (retval, loc)
> && INDIRECT_TYPE_P (valtype))
> - retval = build2 (COMPOUND_EXPR, TREE_TYPE (retval), retval,
> - build_zero_cst (TREE_TYPE (retval)));
> + /* Suppress the Wdangling-pointer warning in the return statement
> + that would otherwise occur. */
> + *no_warning = true;
> }
>
> if (processing_template_decl)
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C
> index a4bc71d890a..ad3ef579f63 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C
> @@ -1,11 +1,11 @@
> // { dg-do compile { target c++14 } }
> // { dg-options "-Wno-return-local-addr" }
>
> -constexpr const int& id(int x) { return x; }
> +constexpr const int& id(int x) { return x; } // { dg-message "note: declared here" }
>
> constexpr bool test() {
> const int& y = id(3);
> return y == 3;
> }
>
> -constexpr bool x = test(); // { dg-error "" }
> +constexpr bool x = test(); // { dg-error "accessing object outside its lifetime" }
> diff --git a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
> index fae8b7e766f..ec8e241d83e 100644
> --- a/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
> +++ b/gcc/testsuite/g++.dg/warn/Wreturn-local-addr-6.C
> @@ -24,6 +24,3 @@ return_addr_local_as_intref (void)
>
> return (const intptr_t&)a; // { dg-warning "\\\[-Wreturn-local-addr]" } */
> }
> -
> -/* Verify that the return value has been replaced with zero:
> - { dg-final { scan-tree-dump-times "return 0;" 2 "optimized" } } */
> --
> 2.34.1
>
>
next prev parent reply other threads:[~2023-06-23 16:58 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-29 2:32 [PATCH v2 0/3] c++: Track lifetimes in constant evaluation [PR70331,...] Nathaniel Shead
2023-03-29 2:32 ` [PATCH v2 1/3] c++: Track lifetimes in constant evaluation [PR70331,PR96630,PR98675] Nathaniel Shead
2023-06-23 16:43 ` [PATCH v2 1/3] c++: Track lifetimes in constant evaluation [PR70331, PR96630, PR98675] Patrick Palka
2023-06-24 14:02 ` Nathaniel Shead
2023-06-26 19:37 ` Patrick Palka
2023-06-30 15:45 ` Nathaniel Shead
2023-03-29 2:32 ` [PATCH v2 2/3] c++: Improve constexpr error for dangling local variables Nathaniel Shead
2023-06-23 16:57 ` Patrick Palka [this message]
2023-03-29 2:32 ` [PATCH v2 3/3] c++: Improve location information in constexpr evaluation Nathaniel Shead
2023-06-23 17:09 ` Patrick Palka
2023-06-30 15:50 ` Nathaniel Shead
2023-05-08 10:48 ` [PATCH v2 0/3] c++: Track lifetimes in constant evaluation [PR70331,...] Nathaniel Shead
2023-06-13 10:11 ` Nathaniel Shead
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c3a608de-e9c5-05c7-ce92-2632b89f7c49@idea \
--to=ppalka@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=nathanieloshead@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).