From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by sourceware.org (Postfix) with ESMTPS id BBE003858436 for ; Tue, 20 Dec 2022 19:39:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BBE003858436 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-ej1-x632.google.com with SMTP id bj12so31601507ejb.13 for ; Tue, 20 Dec 2022 11:39:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:in-reply-to:cc:references:message-id:date:subject:mime-version :from:content-transfer-encoding:from:to:cc:subject:date:message-id :reply-to; bh=12ZhbOtT+awtz28JhVvyAFztE7jYCkrLkQQ11NZFJVU=; b=B/y4OExiArsd8/Hxjm6DVf/BGFzrgj6GqzQ4B4eGPMeFN4exNUkeEpj3myCiYCqOWn cSvO3gXeJdnH1l8SbroXobmYpOZl2UR/1G7bI/LOCoroRrGfEpGr5KSCWE7WcgCK/K1k QENlWEecovVFmR9ak90OxvqBwIxLtBw5vGr9id4nsf2hpM5GzZI1en8TxE+q3L1tlHWt h2mEXa6QlBgXJ7uIbl4QVqwVVrB5bj7bWIVEnovhhXUrWw2mY7klnA0k35JIZr+Y9M0h VCGBkQIEiUyRWf4UzlslBf7fde1aj0XEySIUiDLUYfsPptt8/rKe9Y9/Sm3d0Zm47Ke8 t/Pg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:in-reply-to:cc:references:message-id:date:subject:mime-version :from:content-transfer-encoding:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=12ZhbOtT+awtz28JhVvyAFztE7jYCkrLkQQ11NZFJVU=; b=765FoE/QR81XtzGPD//L3UtlPheoXPJA/4WMYZPlwDOm5txP+RwHMM6tjAayGzN3nd ev14kdlzdeNDW89M9oSHXBoFwMim+z5tMrPETtiO7xal43GJ4Y+U1OMIHbHSUqaTYTm8 3qUmMt8/sGodaT2qVinpTC3WHBneXqoc/QOrEtJbyPBAUu7+rcOt5ocY3RSeb5nBgNcR 3EJ0ul0khR5BN+JbJIMbpmIXM7aNnruzFde6eIvemEB4x3FXfEab0esRUXy+TmnUo668 Spu1r2/cNvSjEEZiyodjG+JEVH3TxmdeLslnsSvDE8die4y6lSGeKoTuuiNwZ15RF7ss nNuA== X-Gm-Message-State: ANoB5pmZoeEFk3Mg77SzXANSQrXak/4uArFkJmx4wf0hQiNlScvmRM5J zSl8JOvo5yuwZG/4YGYweBhLH7FjGrQ= X-Google-Smtp-Source: AA0mqf4n+Hn9z1ddTtoVV9Cb4VquMGgXK8s2OtLCoiuaEu3s7uNhcmSoSbpQg8Gwh8RxswNBQ6ryeA== X-Received: by 2002:a17:907:765a:b0:7c0:e305:a282 with SMTP id kj26-20020a170907765a00b007c0e305a282mr39357785ejc.59.1671565163610; Tue, 20 Dec 2022 11:39:23 -0800 (PST) Received: from smtpclient.apple (dynamic-077-004-026-120.77.4.pool.telefonica.de. [77.4.26.120]) by smtp.gmail.com with ESMTPSA id w5-20020a17090652c500b007c4fbb79535sm6018585ejn.82.2022.12.20.11.39.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 20 Dec 2022 11:39:23 -0800 (PST) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable From: Richard Biener Mime-Version: 1.0 (1.0) Subject: Re: [PATCH RFA(tree)] c++: source position of lambda captures [PR84471] Date: Tue, 20 Dec 2022 20:39:12 +0100 Message-Id: References: Cc: gcc-patches@gcc.gnu.org In-Reply-To: To: Jason Merrill X-Mailer: iPhone Mail (20C65) X-Spam-Status: No, score=-10.5 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: > Am 20.12.2022 um 18:38 schrieb Jason Merrill : >=20 > =EF=BB=BFOn 12/20/22 07:07, Richard Biener wrote: >>> On Fri, Dec 2, 2022 at 4:46 PM Jason Merrill via Gcc-patches >>> wrote: >>>=20 >>> Tested x86_64-pc-linux-gnu, OK for trunk? >>>=20 >>> -- 8< -- >>>=20 >>> 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 debugg= er >>> jumping back and forth for both lambdas and structured bindings. >>>=20 >>> Rather than fix all the uses, it seems simplest to remove any EXPR_LOCAT= ION >>> 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. >>>=20 >>> PR c++/84471 >>> PR c++/107504 >>>=20 >>> gcc/cp/ChangeLog: >>>=20 >>> * coroutines.cc (transform_local_var_uses): Don't >>> specify a location for DECL_VALUE_EXPR. >>> * decl.cc (cp_finish_decomp): Likewise. >>>=20 >>> gcc/ChangeLog: >>>=20 >>> * tree.cc (decl_value_expr_insert): Clear EXPR_LOCATION. >>>=20 >>> gcc/testsuite/ChangeLog: >>>=20 >>> * 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 >>>=20 >>> 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_subt= ree, void *d) >>> =3D lookup_member (lvd->coro_frame_type, local_var.field_id,= >>> /*protect=3D*/1, /*want_type=3D*/0, >>> tf_warning_or_error); >>> - tree fld_idx =3D build3_loc (lvd->loc, COMPONENT_REF, TREE_TYP= E (lvar), >>> - lvd->actor_frame, fld_ref, NULL_TRE= E); >>> + tree fld_idx =3D build3 (COMPONENT_REF, TREE_TYPE (lvar), >>> + lvd->actor_frame, fld_ref, NULL_TREE); >>> local_var.field_idx =3D fld_idx; >>> SET_DECL_VALUE_EXPR (lvar, fld_idx); >>> DECL_HAS_VALUE_EXPR_P (lvar) =3D 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 i= nt count) >>> if (processing_template_decl) >>> continue; >>> tree t =3D unshare_expr (dexp); >>> - t =3D build4_loc (DECL_SOURCE_LOCATION (v[i]), ARRAY_REF, >>> - eltype, t, size_int (i), NULL_TREE, >>> - NULL_TREE); >>> + t =3D build4 (ARRAY_REF, eltype, t, size_int (i), NULL_TREE, N= ULL_TREE); >>> SET_DECL_VALUE_EXPR (v[i], t); >>> DECL_HAS_VALUE_EXPR_P (v[i]) =3D 1; >>> } >>> @@ -9154,9 +9152,7 @@ cp_finish_decomp (tree decl, tree first, unsigned i= nt count) >>> if (processing_template_decl) >>> continue; >>> tree t =3D unshare_expr (dexp); >>> - t =3D build1_loc (DECL_SOURCE_LOCATION (v[i]), >>> - i ? IMAGPART_EXPR : REALPART_EXPR, eltype, >>> - t); >>> + t =3D build1 (i ? IMAGPART_EXPR : REALPART_EXPR, eltype, t); >>> SET_DECL_VALUE_EXPR (v[i], t); >>> DECL_HAS_VALUE_EXPR_P (v[i]) =3D 1; >>> } >>> @@ -9180,9 +9176,7 @@ cp_finish_decomp (tree decl, tree first, unsigned i= nt count) >>> tree t =3D unshare_expr (dexp); >>> convert_vector_to_array_for_subscript (DECL_SOURCE_LOCATION (v= [i]), >>> &t, size_int (i)); >>> - t =3D build4_loc (DECL_SOURCE_LOCATION (v[i]), ARRAY_REF, >>> - eltype, t, size_int (i), NULL_TREE, >>> - NULL_TREE); >>> + t =3D build4 (ARRAY_REF, eltype, t, size_int (i), NULL_TREE, N= ULL_TREE); >>> SET_DECL_VALUE_EXPR (v[i], t); >>> DECL_HAS_VALUE_EXPR_P (v[i]) =3D 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 =3D 3; >>> - return [&aha] { // { dg-warning "dereferencing pointer '.*' to with= in stale stack frame" } >>> - return aha; >>> + return [&aha] { >>> + return aha; // { dg-warning "dereferencing pointer '.*' to with= in 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 =3D 1; >>> + auto f =3D [&x, &argc](const char* i) { >>> + i +=3D x; >>> + i -=3D argc; >>> + i +=3D 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] =3D f(buf); >>> + if (x !=3D buf) >>> + throw 1; >>> + if (y !=3D 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; >>>=20 >>> + /* 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. >=20 > I think these expressions aren't ever shared in practice, but I agree it's= safer to use the _unshare variant. OK with that change? >=20 >> Maybe it would be easier to handle this in the consumers of the >> DECL_VALUE_EXPR? gimplify_var_or_parm_decl does >=20 > I don't see how auditing all the (many) users of DECL_VALUE_EXPR would be e= asier than doing it in this one place It might do less unsharing. But OK with the _unshare variant. Thanks, Richard=20 >> /* If the decl is an alias for another expression, substitute it now. *= / >> if (DECL_HAS_VALUE_EXPR_P (decl)) >> { >> *expr_p =3D unshare_expr (DECL_VALUE_EXPR (decl)); >> return GS_OK; >> it could also unshare without location. >=20 >=20