From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x330.google.com (mail-ot1-x330.google.com [IPv6:2607:f8b0:4864:20::330]) by sourceware.org (Postfix) with ESMTPS id 9E7043858020 for ; Thu, 15 Apr 2021 13:36:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9E7043858020 Received: by mail-ot1-x330.google.com with SMTP id 65-20020a9d03470000b02902808b4aec6dso17824966otv.6 for ; Thu, 15 Apr 2021 06:36:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=srTj0q55FFmSoMB0xJWaqNlD9kJa73XO3AH42IfCFUg=; b=sI4hvvTorEQbzmZWsG8F8xrj6ZJIg8oqSSyIDMxaLjWzQV9/aZm5e60wbTdr0VM2zS CMtJpioeMs0Dtpqxcvvl1sYKNulbUvAu3jriJf47xF9IXzS6Jl16yR19cFAIIKTgvq7K 26ip5OwWr25slLfR8VyJXmZ8NgqaXUJovN00FSeqK9JuaPlr9kiAm9Kenm2f14I4a1/3 nCWm77zk02sZ5HUHVLNoOTanFbxsx7Wk08VJzpSLdPJOzh9sqS+0u140Qr3Cp44tpx53 AZ0nt94+w3QK6tmkvqworP02FFdWJ3/o1RaszjRyTxCC2KVgaVpVS4gUWHPk7sOsV5+t sIVg== X-Gm-Message-State: AOAM530YrKd5M4EtnpuAVoiPvti1VHpZ6qQxtcoBKXrMyB4LJQyhAOW7 nEfh0yiFflYDkIpDzm+6k+D3V5Vhlmpp7Qw0ql/rzv6DOX/n4Q== X-Google-Smtp-Source: ABdhPJydahWrSbCTXa77mSM43bYy+4Sxb3gpsyx71h1C3StAfUiS38RgbhLRGx5gRppzzw/2LfNbQVCd3RFLtCKMFBY= X-Received: by 2002:a9d:204:: with SMTP id 4mr1565796otb.352.1618493761491; Thu, 15 Apr 2021 06:36:01 -0700 (PDT) MIME-Version: 1.0 References: <20210414001958.344809-1-jason@redhat.com> In-Reply-To: <20210414001958.344809-1-jason@redhat.com> From: Christophe Lyon Date: Thu, 15 Apr 2021 15:35:50 +0200 Message-ID: Subject: Re: [pushed] c++: debug location of variable cleanups [PR88742] To: Jason Merrill Cc: gcc Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Apr 2021 13:36:05 -0000 Hi, On Wed, 14 Apr 2021 at 02:20, Jason Merrill via Gcc-patches 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*))->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 >