> Am 20.12.2022 um 18:38 schrieb Jason Merrill <ja...@redhat.com>:
>
> On 12/20/22 07:07, Richard Biener wrote:
>>> On Fri, Dec 2, 2022 at 4:46 PM Jason Merrill via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> 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
It might do less unsharing. But OK with the _unshare variant.
Thanks,
Richard
>> /* 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.
>
>
Re: [PATCH RFA(tree)] c++: source position of lambda captures [PR84471]
Richard Biener via Gcc-patches Tue, 20 Dec 2022 11:39:37 -0800
- [PATCH RFA(tree)] c++: source position of l... Jason Merrill via Gcc-patches
- Re: [PATCH RFA(tree)] c++: source posi... Jason Merrill via Gcc-patches
- Re: [PATCH PING 2 (tree)] c++: source ... Jason Merrill via Gcc-patches
- Re: [PATCH RFA(tree)] c++: source posi... Richard Biener via Gcc-patches
- Re: [PATCH RFA(tree)] c++: source ... Jason Merrill via Gcc-patches
- Re: [PATCH RFA(tree)] c++: sou... Richard Biener via Gcc-patches
- Re: [PATCH RFA(tree)] c++:... Jason Merrill via Gcc-patches
