From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x133.google.com (mail-lf1-x133.google.com [IPv6:2a00:1450:4864:20::133]) by sourceware.org (Postfix) with ESMTPS id 7FE293858421 for ; Tue, 20 Dec 2022 12:07:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7FE293858421 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lf1-x133.google.com with SMTP id o6so13398625lfi.5 for ; Tue, 20 Dec 2022 04:07:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Rt6mf3wkAII1w+ALh1f7hJSQF2BZngOXh2dhzhUbrtc=; b=entzNs6/DpFyG0aOk96v3yZ8lVxFsNlVu0LyB+nvRfPCCpF1i105mDVyCF+MGO3APu vCMUbxPhidc2oQ/qj9QhgyP3RQlFBlwDRTmBvWIR8HLFwaittB6u1MAiSfXrjgZJ2L2O LMa3M/L0S6ixE+tF0L7AjGml8OJRLKUIqokN/LPIR2iPcAAfxTjjt3RpK4SDnh0kpFwb tEB0222pnyojEQoWS3FtjjWvd04V7xyfq7U2Wn1pXqannDENMUslFkVAa5tsyp7/vVTW ZmYBfP0OWTpI8SHCaIuqnqbjpGIj7AJz/b9Bx3TeCF0wqlUfh2X+XJxnW4oBlfZAsLvW m4Bg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Rt6mf3wkAII1w+ALh1f7hJSQF2BZngOXh2dhzhUbrtc=; b=vt8lmDjBswBZtBoCJlQgYP2hPl8CXx9OzlmycJ902ZkOstlt8btI2OGj3VVdBd09lL WMfgvoYi7Y0ClkAoVHUC4prmcBtFB287JTO/c1HTey4YAeiX+DAw93PLu4g/i4HZP2nt eQH30m8sGkfvsPqGjelMcvxWPEnqwbhm+Y68b1RK7IC+KEsDgyKSbzanqDTLjIXN+cVy w9lJGnByQjs2s4x7I8xE/X58GAyOAi8DFB+MFb3TsbzYXgDxCEoz2ceVTKkzDo66/JhJ rJC0rU82PY5D/6Zl1aRwaZJDc4BV/Xn/qgQXk1nZOV4TkMB59xgh/sa0geY2EMxtcf0f Q6pw== X-Gm-Message-State: AFqh2kqHnLR4pUU2VA96r0LoMuYai0PNuEd9xYx3swOWpC6dK+bJXCuN gmX2JuTUHn5Ixh4ii6Xtfq51idP2fJ1Jnk3KSeQ= X-Google-Smtp-Source: AMrXdXvlkmITVXkE6B2YeOyaDDPCFviaJziDKka64diyseVNCZbXslf5DzR/dZuFau/QtYpaqNWhGxMo48vkAf+IFig= X-Received: by 2002:a05:6512:ac9:b0:4ba:caf8:a948 with SMTP id n9-20020a0565120ac900b004bacaf8a948mr1111969lfu.169.1671538075725; Tue, 20 Dec 2022 04:07:55 -0800 (PST) MIME-Version: 1.0 References: <20221202154512.310755-1-jason@redhat.com> In-Reply-To: <20221202154512.310755-1-jason@redhat.com> From: Richard Biener Date: Tue, 20 Dec 2022 13:07:43 +0100 Message-ID: Subject: Re: [PATCH RFA(tree)] c++: source position of lambda captures [PR84471] To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, Dec 2, 2022 at 4:46 PM Jason Merrill via Gcc-patches wrote: > > Tested x86_64-pc-linux-gnu, OK for trunk? > > -- 8< -- > > If the DECL_VALUE_EXPR of a VAR_DECL has EXPR_LOCATION set, then any use of > that variable looks like it has that location, which leads to the debugger > jumping back and forth for both lambdas and structured bindings. > > Rather than fix all the uses, it seems simplest to remove any EXPR_LOCATION > when setting DECL_VALUE_EXPR. So the cp/ hunks aren't necessary, but it > seems cleaner not to work to add a location that will immediately get > stripped. > > PR c++/84471 > PR c++/107504 > > gcc/cp/ChangeLog: > > * coroutines.cc (transform_local_var_uses): Don't > specify a location for DECL_VALUE_EXPR. > * decl.cc (cp_finish_decomp): Likewise. > > gcc/ChangeLog: > > * tree.cc (decl_value_expr_insert): Clear EXPR_LOCATION. > > gcc/testsuite/ChangeLog: > > * g++.dg/tree-ssa/value-expr1.C: New test. > * g++.dg/tree-ssa/value-expr2.C: New test. > * g++.dg/analyzer/pr93212.C: Move warning. > --- > gcc/cp/coroutines.cc | 4 ++-- > gcc/cp/decl.cc | 12 +++------- > gcc/testsuite/g++.dg/analyzer/pr93212.C | 4 ++-- > gcc/testsuite/g++.dg/tree-ssa/value-expr1.C | 16 +++++++++++++ > gcc/testsuite/g++.dg/tree-ssa/value-expr2.C | 26 +++++++++++++++++++++ > gcc/tree.cc | 3 +++ > 6 files changed, 52 insertions(+), 13 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/value-expr1.C > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/value-expr2.C > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index 01a3e831ee5..a72bd6bbef0 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -2047,8 +2047,8 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d) > = lookup_member (lvd->coro_frame_type, local_var.field_id, > /*protect=*/1, /*want_type=*/0, > tf_warning_or_error); > - tree fld_idx = build3_loc (lvd->loc, COMPONENT_REF, TREE_TYPE (lvar), > - lvd->actor_frame, fld_ref, NULL_TREE); > + tree fld_idx = build3 (COMPONENT_REF, TREE_TYPE (lvar), > + lvd->actor_frame, fld_ref, NULL_TREE); > local_var.field_idx = fld_idx; > SET_DECL_VALUE_EXPR (lvar, fld_idx); > DECL_HAS_VALUE_EXPR_P (lvar) = true; > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 7af0b05d5f8..59e21581503 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -9133,9 +9133,7 @@ cp_finish_decomp (tree decl, tree first, unsigned int count) > if (processing_template_decl) > continue; > tree t = unshare_expr (dexp); > - t = build4_loc (DECL_SOURCE_LOCATION (v[i]), ARRAY_REF, > - eltype, t, size_int (i), NULL_TREE, > - NULL_TREE); > + t = build4 (ARRAY_REF, eltype, t, size_int (i), NULL_TREE, NULL_TREE); > SET_DECL_VALUE_EXPR (v[i], t); > DECL_HAS_VALUE_EXPR_P (v[i]) = 1; > } > @@ -9154,9 +9152,7 @@ cp_finish_decomp (tree decl, tree first, unsigned int count) > if (processing_template_decl) > continue; > tree t = unshare_expr (dexp); > - t = build1_loc (DECL_SOURCE_LOCATION (v[i]), > - i ? IMAGPART_EXPR : REALPART_EXPR, eltype, > - t); > + t = build1 (i ? IMAGPART_EXPR : REALPART_EXPR, eltype, t); > SET_DECL_VALUE_EXPR (v[i], t); > DECL_HAS_VALUE_EXPR_P (v[i]) = 1; > } > @@ -9180,9 +9176,7 @@ cp_finish_decomp (tree decl, tree first, unsigned int count) > tree t = unshare_expr (dexp); > convert_vector_to_array_for_subscript (DECL_SOURCE_LOCATION (v[i]), > &t, size_int (i)); > - t = build4_loc (DECL_SOURCE_LOCATION (v[i]), ARRAY_REF, > - eltype, t, size_int (i), NULL_TREE, > - NULL_TREE); > + t = build4 (ARRAY_REF, eltype, t, size_int (i), NULL_TREE, NULL_TREE); > SET_DECL_VALUE_EXPR (v[i], t); > DECL_HAS_VALUE_EXPR_P (v[i]) = 1; > } > diff --git a/gcc/testsuite/g++.dg/analyzer/pr93212.C b/gcc/testsuite/g++.dg/analyzer/pr93212.C > index 41507e2b837..1029e8d547b 100644 > --- a/gcc/testsuite/g++.dg/analyzer/pr93212.C > +++ b/gcc/testsuite/g++.dg/analyzer/pr93212.C > @@ -4,8 +4,8 @@ > auto lol() > { > int aha = 3; > - return [&aha] { // { dg-warning "dereferencing pointer '.*' to within stale stack frame" } > - return aha; > + return [&aha] { > + return aha; // { dg-warning "dereferencing pointer '.*' to within stale stack frame" } > }; > /* TODO: may be worth special-casing the reporting of dangling > references from lambdas, to highlight the declaration, and maybe fix > diff --git a/gcc/testsuite/g++.dg/tree-ssa/value-expr1.C b/gcc/testsuite/g++.dg/tree-ssa/value-expr1.C > new file mode 100644 > index 00000000000..946ccc3bd97 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/tree-ssa/value-expr1.C > @@ -0,0 +1,16 @@ > +// PR c++/84471 > +// { dg-do compile { target c++11 } } > +// { dg-additional-options -fdump-tree-gimple-lineno } > +// { dg-final { scan-tree-dump-not {value-expr: \[} "gimple" } } > + > +int main(int argc, char**) > +{ > + int x = 1; > + auto f = [&x, &argc](const char* i) { > + i += x; > + i -= argc; > + i += argc - x; > + return i; > + }; > + f(" "); > +} > diff --git a/gcc/testsuite/g++.dg/tree-ssa/value-expr2.C b/gcc/testsuite/g++.dg/tree-ssa/value-expr2.C > new file mode 100644 > index 00000000000..4d00498f214 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/tree-ssa/value-expr2.C > @@ -0,0 +1,26 @@ > +// PR c++/107504 > +// { dg-do compile { target c++17 } } > +// { dg-additional-options -fdump-tree-gimple-lineno } > +// { dg-final { scan-tree-dump-not {value-expr: \[} "gimple" } } > + > +struct S > +{ > + void* i; > + int j; > +}; > + > +S f(char* p) > +{ > + return {p, 1}; > +} > + > +int main() > +{ > + char buf[1]; > + auto [x, y] = f(buf); > + if (x != buf) > + throw 1; > + if (y != 1) > + throw 2; > + return 0; > +} > diff --git a/gcc/tree.cc b/gcc/tree.cc > index 254b2373dcf..836c51cd4d5 100644 > --- a/gcc/tree.cc > +++ b/gcc/tree.cc > @@ -5862,6 +5862,9 @@ decl_value_expr_insert (tree from, tree to) > { > struct tree_decl_map *h; > > + /* Uses of FROM shouldn't look like they happen at the location of TO. */ > + protected_set_expr_location (to, UNKNOWN_LOCATION); > + Doesn't that mean we'd eventually want unshare_expr_without_location or similar here? Or rather maybe set the location of TO to that of FROM? That said, this modifies FROM in place - we have protected_set_expr_location_unshare (would need to be exported from fold-const.cc) to avoid clobbering a possibly shared tree. Maybe it would be easier to handle this in the consumers of the DECL_VALUE_EXPR? gimplify_var_or_parm_decl does /* If the decl is an alias for another expression, substitute it now. */ if (DECL_HAS_VALUE_EXPR_P (decl)) { *expr_p = unshare_expr (DECL_VALUE_EXPR (decl)); return GS_OK; it could also unshare without location. > h = ggc_alloc (); > h->base.from = from; > h->to = to; > > base-commit: 70596a0fb2a2ec072e1e97e37616e05041dfa4e5 > -- > 2.31.1 >