From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id CF3A23858421 for ; Tue, 20 Dec 2022 17:38:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CF3A23858421 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1671557906; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bgI51fOxFJMWSKfb4AgZqM1PoAOKCbKpPF1EUpYHkJI=; b=VWLa3aBGrxT2vX+x0BKnNZj48pFr6dbNYJKxadX/3067xY9T5SPASgUUxpcWbDtjfOe366 wQKEAqq1aRN6yVxX73BNNNNx7l8AwWnKh+miCymCtEA43VLXYuvnLnNQEF9V5wIzNBmnfm p1QfdurNxStzl7IlEic7HHkgkbbf+uM= Received: from mail-yw1-f199.google.com (mail-yw1-f199.google.com [209.85.128.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-198-nMC0rV4-OX2O__sMZTcIjQ-1; Tue, 20 Dec 2022 12:38:23 -0500 X-MC-Unique: nMC0rV4-OX2O__sMZTcIjQ-1 Received: by mail-yw1-f199.google.com with SMTP id 00721157ae682-352e29ff8c2so151224987b3.21 for ; Tue, 20 Dec 2022 09:38:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bgI51fOxFJMWSKfb4AgZqM1PoAOKCbKpPF1EUpYHkJI=; b=hV6EZql/+6mjZEHConJMpBlHdisdErMrNUIqj2305LFgB1n8hM+zHX+GGC++YgSP4x 4VuNn1cwFpnb0y4tgIDEM8lH44AHuf7SpgtYv3RzRG86lOGagrgGer6VTGMzeqjK0QzA 33PGUWS/maJLUB19XS2j/q691+LrMmX2vAvHrw5HMSBKO4QR6cN3svBjpIdy4sdUAqf4 Bgxl/NEXqi+h/nGRs30aVx8mrXkuL/6Wydvl0pZYc+WiQsYe8SdVfqXxrIVVawYmUEni tb0DGnO36ol92MA7IqpDiiIhzP1lcbRr2LYzK5JHpiSE0cCdiwoBeQ0uuvlSqVzqRdJw Viyw== X-Gm-Message-State: ANoB5pl4tYUa9I8vrYJrKa88mofF8sB5V6Gy+mYCoffuB/XVX0x35ObA NMBBEgUmcSPxZ4MKbTRhCh6c1ppPtbrG2ihUXdp2+HUoVepTxu3oYdBXVdFREYOvYhbQ6b9kph5 Oo/7xhMNJf/kcmmsU6Q== X-Received: by 2002:a25:cec6:0:b0:6fc:da94:4409 with SMTP id x189-20020a25cec6000000b006fcda944409mr42317699ybe.17.1671557902681; Tue, 20 Dec 2022 09:38:22 -0800 (PST) X-Google-Smtp-Source: AA0mqf7dKBKntMIm1oXm0RCnoPAgWNV8OG4WUvb5VUAROvc3kgfmLuK3+pB245/xci58OOoM/oJmsw== X-Received: by 2002:a25:cec6:0:b0:6fc:da94:4409 with SMTP id x189-20020a25cec6000000b006fcda944409mr42317671ybe.17.1671557902295; Tue, 20 Dec 2022 09:38:22 -0800 (PST) Received: from [192.168.1.108] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id s81-20020a37a954000000b006eeb3165554sm9066950qke.19.2022.12.20.09.38.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 20 Dec 2022 09:38:21 -0800 (PST) Message-ID: Date: Tue, 20 Dec 2022 12:38:20 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH RFA(tree)] c++: source position of lambda captures [PR84471] To: Richard Biener Cc: gcc-patches@gcc.gnu.org References: <20221202154512.310755-1-jason@redhat.com> From: Jason Merrill In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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 12/20/22 07:07, Richard Biener wrote: > 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. 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? > Maybe it would be easier to handle this in the consumers of the > DECL_VALUE_EXPR? gimplify_var_or_parm_decl does I don't see how auditing all the (many) users of DECL_VALUE_EXPR would be easier than doing it in this one place? > /* 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.