From: Christophe Lyon <christophe.lyon@linaro.org>
To: Jason Merrill <jason@redhat.com>
Cc: gcc Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [pushed] c++: debug location of variable cleanups [PR88742]
Date: Thu, 15 Apr 2021 15:35:50 +0200 [thread overview]
Message-ID: <CAKdteOYKARgTCmH5+sn4kWGCAsn9fbD6ugwrdavcLvQu9ygeMw@mail.gmail.com> (raw)
In-Reply-To: <20210414001958.344809-1-jason@redhat.com>
Hi,
On Wed, 14 Apr 2021 at 02:20, Jason Merrill via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> PR49951 complained about the debugger jumping back to the declaration of a
> local variable when we run its destructor. That was fixed in 4.7, but broke
> again in 4.8. PR58123 fixed an inconsistency in the behavior, but not the
> jumping around. This patch addresses the issue by setting EXPR_LOCATION on
> a cleanup destructor call to the location of the closing brace of the
> compound-statement, or whatever token ends the scope of the variable.
>
> The change to cp_parser_compound_statement is so input_location is the }
> rather than the ; of the last substatement.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.
>
> gcc/cp/ChangeLog:
>
> PR c++/88742
> PR c++/49951
> PR c++/58123
> * semantics.c (set_cleanup_locs): New.
> (do_poplevel): Call it.
> * parser.c (cp_parser_compound_statement): Consume the }
> before finish_compound_stmt.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/88742
> * g++.dg/debug/cleanup1.C: New test.
> * c-c++-common/Wimplicit-fallthrough-6.c: Adjust diagnostic line.
> * c-c++-common/Wimplicit-fallthrough-7.c: Likewise.
> * g++.dg/cpp2a/constexpr-dtor3.C: Likewise.
This change is causing a regression on arm:
FAIL: g++.dg/cpp2a/constexpr-dtor3.C -std=c++2a (test for warnings, line 154)
the logs contain:
[...]
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:131:22: in 'constexpr'
expansion of '((W5*)(& w13))->W5::~W5()'
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:72:34: error: inline
assembly is not a constant expression
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:72:34: note: only
unevaluated inline assembly is allowed in a 'constexpr' function in
C++20
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:133:35: in 'constexpr'
expansion of '((W6*)<anonymous>)->W6::~W6()'
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:80:34: error: inline
assembly is not a constant expression
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:80:34: note: only
unevaluated inline assembly is allowed in a 'constexpr' function in
C++20
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C: At global scope:
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:156:23: in 'constexpr'
expansion of 'f4()'
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:152:12: in 'constexpr'
expansion of '(& w13)->W7::~W7()'
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:88:34: error: inline
assembly is not a constant expression
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:88:34: note: only
unevaluated inline assembly is allowed in a 'constexpr' function in
C++20
/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C:165:23: in 'constexpr'
expansion of 'f5()'
[...]
Can you check?
Thanks
Christophe
> * g++.dg/ext/constexpr-attr-cleanup1.C: Likewise.
> * g++.dg/tm/inherit2.C: Likewise.
> * g++.dg/tm/unsafe1.C: Likewise.
> * g++.dg/warn/Wimplicit-fallthrough-1.C: Likewise.
> * g++.dg/gcov/gcov-2.C: Adjust coverage counts.
> ---
> gcc/cp/parser.c | 5 ++-
> gcc/cp/semantics.c | 19 +++++++++
> .../c-c++-common/Wimplicit-fallthrough-6.c | 16 ++++----
> .../c-c++-common/Wimplicit-fallthrough-7.c | 4 +-
> gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C | 4 +-
> gcc/testsuite/g++.dg/debug/cleanup1.C | 41 +++++++++++++++++++
> .../g++.dg/ext/constexpr-attr-cleanup1.C | 4 +-
> gcc/testsuite/g++.dg/gcov/gcov-2.C | 4 +-
> gcc/testsuite/g++.dg/tm/inherit2.C | 4 +-
> gcc/testsuite/g++.dg/tm/unsafe1.C | 4 +-
> .../g++.dg/warn/Wimplicit-fallthrough-1.C | 4 +-
> 11 files changed, 85 insertions(+), 24 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/debug/cleanup1.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 8b7801b2be7..aec3aa3587f 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -12126,11 +12126,12 @@ cp_parser_compound_statement (cp_parser *parser, tree in_statement_expr,
> if (function_body)
> maybe_splice_retval_cleanup (compound_stmt);
>
> - /* Finish the compound-statement. */
> - finish_compound_stmt (compound_stmt);
> /* Consume the `}'. */
> braces.require_close (parser);
>
> + /* Finish the compound-statement. */
> + finish_compound_stmt (compound_stmt);
> +
> return compound_stmt;
> }
>
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index 8eaaaefe2d6..125772238d3 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -602,6 +602,22 @@ add_decl_expr (tree decl)
> add_stmt (r);
> }
>
> +/* Set EXPR_LOCATION of the cleanups of any CLEANUP_STMT in STMTS to LOC. */
> +
> +static void
> +set_cleanup_locs (tree stmts, location_t loc)
> +{
> + if (TREE_CODE (stmts) == CLEANUP_STMT)
> + {
> + protected_set_expr_location (CLEANUP_EXPR (stmts), loc);
> + set_cleanup_locs (CLEANUP_BODY (stmts), loc);
> + }
> + else if (TREE_CODE (stmts) == STATEMENT_LIST)
> + for (tree_stmt_iterator i = tsi_start (stmts);
> + !tsi_end_p (i); tsi_next (&i))
> + set_cleanup_locs (tsi_stmt (i), loc);
> +}
> +
> /* Finish a scope. */
>
> tree
> @@ -614,6 +630,9 @@ do_poplevel (tree stmt_list)
>
> stmt_list = pop_stmt_list (stmt_list);
>
> + /* input_location is the last token of the scope, usually a }. */
> + set_cleanup_locs (stmt_list, input_location);
> +
> if (!processing_template_decl)
> {
> stmt_list = c_build_bind_expr (input_location, block, stmt_list);
> diff --git a/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-6.c b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-6.c
> index 32d5febda83..9593f670709 100644
> --- a/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-6.c
> +++ b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-6.c
> @@ -121,8 +121,8 @@ L1:
> int j = 0;
> bar (j);
> if (j == 8)
> - return; /* { dg-warning "statement may fall through" "" { target c++ } } */
> - }
> + return;
> + } /* { dg-warning "statement may fall through" "" { target c++ } } */
> case 2:
> bar (99);
> }
> @@ -151,8 +151,8 @@ L1:
> if (j == 8)
> bar (1);
> else
> - return; /* { dg-warning "statement may fall through" "" { target c++ } } */
> - }
> + return;
> + } /* { dg-warning "statement may fall through" "" { target c++ } } */
> case 2:
> bar (99);
> }
> @@ -181,8 +181,8 @@ L1:
> if (j == 8)
> bar (1);
> else
> - bar (2); /* { dg-warning "statement may fall through" "" { target c++ } } */
> - }
> + bar (2);
> + } /* { dg-warning "statement may fall through" "" { target c++ } } */
> case 2:
> bar (99);
> }
> @@ -281,8 +281,8 @@ L1:
> case 1:
> { /* { dg-warning "statement may fall through" "" { target c } } */
> int j = 9;
> - switch (j); /* { dg-warning "statement may fall through" "" { target c++ } } */
> - }
> + switch (j);
> + } /* { dg-warning "statement may fall through" "" { target c++ } } */
> case 2:
> bar (99);
> }
> diff --git a/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-7.c b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-7.c
> index a602216fa73..343b6250807 100644
> --- a/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-7.c
> +++ b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-7.c
> @@ -22,8 +22,8 @@ f (int i)
> {
> case 1:
> { /* { dg-warning "statement may fall through" "" { target c } } */
> - int a[i]; /* { dg-warning "statement may fall through" "" { target c++ } } */
> - }
> + int a[i];
> + } /* { dg-warning "statement may fall through" "" { target c++ } } */
> case 2:
> bar (99);
> }
> diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C
> index 193ae93933d..7700bb7b036 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-dtor3.C
> @@ -149,9 +149,9 @@ constexpr int x3 = f3 ();
> constexpr int
> f4 ()
> {
> - W7 w13 = 5; // { dg-message "in 'constexpr' expansion of" }
> + W7 w13 = 5;
> return 0;
> -}
> +} // { dg-message "in 'constexpr' expansion of" }
>
> constexpr int x4 = f4 (); // { dg-message "in 'constexpr' expansion of" }
>
> diff --git a/gcc/testsuite/g++.dg/debug/cleanup1.C b/gcc/testsuite/g++.dg/debug/cleanup1.C
> new file mode 100644
> index 00000000000..ae0515aef7b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/cleanup1.C
> @@ -0,0 +1,41 @@
> +// PR c++/88742
> +// { dg-additional-options -fdump-tree-gimple-lineno }
> +
> +
> +class C {
> +public:
> + C() {}
> + ~C() {}
> + operator int() { return 1; }
> +};
> +
> +int main() {
> + C a;
> + C b;
> + C c;
> +
> + if (C e = C())
> + {
> + if (C d = C())
> + {
> + }
> + else
> + {
> + return 42;
> + } // { dg-final { scan-tree-dump-times ":25:9. C::~C" 1 "gimple" } }
> + } // { dg-final { scan-tree-dump-times ":26:5. C::~C" 1 "gimple" } }
> +
> + while (C f = C())
> + {
> + break;
> + } // { dg-final { scan-tree-dump-times ":31:5. C::~C" 1 "gimple" } }
> +
> + for (C h = C(); C i = C(); )
> + break; // { dg-final { scan-tree-dump-times ":34:10. C::~C" 2 "gimple" } }
> +
> + switch (C g = C())
> + {
> + default:
> + break;
> + } // { dg-final { scan-tree-dump-times ":40:5. C::~C" 1 "gimple" } }
> +} // { dg-final { scan-tree-dump-times ":41:1. C::~C" 3 "gimple" } }
> diff --git a/gcc/testsuite/g++.dg/ext/constexpr-attr-cleanup1.C b/gcc/testsuite/g++.dg/ext/constexpr-attr-cleanup1.C
> index b6e7c691367..ab5fd17e815 100644
> --- a/gcc/testsuite/g++.dg/ext/constexpr-attr-cleanup1.C
> +++ b/gcc/testsuite/g++.dg/ext/constexpr-attr-cleanup1.C
> @@ -15,9 +15,9 @@ cleanup2 (int *x)
> constexpr bool
> foo ()
> {
> - int a __attribute__((cleanup (cleanup))) = 1; // { dg-message "in 'constexpr' expansion of" }
> + int a __attribute__((cleanup (cleanup))) = 1;
> return true;
> -}
> +} // { dg-message "in 'constexpr' expansion of" }
>
> constexpr bool
> bar ()
> diff --git a/gcc/testsuite/g++.dg/gcov/gcov-2.C b/gcc/testsuite/g++.dg/gcov/gcov-2.C
> index 6d002f5d2cd..2b4cdd844e9 100644
> --- a/gcc/testsuite/g++.dg/gcov/gcov-2.C
> +++ b/gcc/testsuite/g++.dg/gcov/gcov-2.C
> @@ -20,9 +20,9 @@ private:
>
> void foo()
> {
> - C c; /* count(2) */
> + C c; /* count(1) */
> c.seti (1); /* count(1) */
> -}
> +} /* count(1) */
>
> int main()
> {
> diff --git a/gcc/testsuite/g++.dg/tm/inherit2.C b/gcc/testsuite/g++.dg/tm/inherit2.C
> index 3b696a9ffb6..366f9b30e90 100644
> --- a/gcc/testsuite/g++.dg/tm/inherit2.C
> +++ b/gcc/testsuite/g++.dg/tm/inherit2.C
> @@ -26,8 +26,8 @@ int main()
> B b; // ok
> D1 d1; // ok
> B& b1 = d1;
> - D2 x; // { dg-error "" "destructor of D2 is not transaction-safe" }
> + D2 x;
> b1.f(); // ok, calls D1::f()
> delete b2; // undefined behavior: calls unsafe destructor of D2
> - }
> + } // { dg-error "" "destructor of D2 is not transaction-safe" }
> }
> diff --git a/gcc/testsuite/g++.dg/tm/unsafe1.C b/gcc/testsuite/g++.dg/tm/unsafe1.C
> index 49dd564894a..710fb1a3031 100644
> --- a/gcc/testsuite/g++.dg/tm/unsafe1.C
> +++ b/gcc/testsuite/g++.dg/tm/unsafe1.C
> @@ -5,8 +5,8 @@ struct S {
> virtual ~S();
> };
> void f() transaction_safe {
> - S s; // { dg-error "unsafe" "invocation of unsafe destructor" }
> -}
> + S s;
> +} // { dg-error "unsafe" "invocation of unsafe destructor" }
>
> int g(int x) { // is transaction-safe
> if (x <= 0)
> diff --git a/gcc/testsuite/g++.dg/warn/Wimplicit-fallthrough-1.C b/gcc/testsuite/g++.dg/warn/Wimplicit-fallthrough-1.C
> index 053ed6885c5..b5ebd3daaad 100644
> --- a/gcc/testsuite/g++.dg/warn/Wimplicit-fallthrough-1.C
> +++ b/gcc/testsuite/g++.dg/warn/Wimplicit-fallthrough-1.C
> @@ -25,8 +25,8 @@ fn2 ()
> switch (0) {
> case 0:
> {
> - A b; // { dg-warning "statement may fall through" }
> - }
> + A b;
> + } // { dg-warning "statement may fall through" }
> default:
> a = 0;
> }
>
> base-commit: 6d0d35d518a12ee43c1fbd77df73a66d02305a69
> --
> 2.27.0
>
prev parent reply other threads:[~2021-04-15 13:36 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-14 0:19 Jason Merrill
2021-04-15 13:35 ` Christophe Lyon [this message]
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=CAKdteOYKARgTCmH5+sn4kWGCAsn9fbD6ugwrdavcLvQu9ygeMw@mail.gmail.com \
--to=christophe.lyon@linaro.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.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).