* [PATCH RFA(tree)] c++: source position of lambda captures [PR84471]
@ 2022-12-02 15:45 Jason Merrill
2022-12-08 18:30 ` Jason Merrill
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jason Merrill @ 2022-12-02 15:45 UTC (permalink / raw)
To: gcc-patches
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);
+
h = ggc_alloc<tree_decl_map> ();
h->base.from = from;
h->to = to;
base-commit: 70596a0fb2a2ec072e1e97e37616e05041dfa4e5
--
2.31.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFA(tree)] c++: source position of lambda captures [PR84471]
2022-12-02 15:45 [PATCH RFA(tree)] c++: source position of lambda captures [PR84471] Jason Merrill
@ 2022-12-08 18:30 ` Jason Merrill
2022-12-19 16:00 ` [PATCH PING 2 (tree)] " Jason Merrill
2022-12-20 12:07 ` [PATCH RFA(tree)] " Richard Biener
2 siblings, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2022-12-08 18:30 UTC (permalink / raw)
To: gcc-patches
Ping.
On 12/2/22 10:45, Jason Merrill 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);
> +
> h = ggc_alloc<tree_decl_map> ();
> h->base.from = from;
> h->to = to;
>
> base-commit: 70596a0fb2a2ec072e1e97e37616e05041dfa4e5
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH PING 2 (tree)] c++: source position of lambda captures [PR84471]
2022-12-02 15:45 [PATCH RFA(tree)] c++: source position of lambda captures [PR84471] Jason Merrill
2022-12-08 18:30 ` Jason Merrill
@ 2022-12-19 16:00 ` Jason Merrill
2022-12-20 12:07 ` [PATCH RFA(tree)] " Richard Biener
2 siblings, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2022-12-19 16:00 UTC (permalink / raw)
To: gcc-patches
On 12/2/22 10:45, Jason Merrill 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);
> +
> h = ggc_alloc<tree_decl_map> ();
> h->base.from = from;
> h->to = to;
>
> base-commit: 70596a0fb2a2ec072e1e97e37616e05041dfa4e5
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFA(tree)] c++: source position of lambda captures [PR84471]
2022-12-02 15:45 [PATCH RFA(tree)] c++: source position of lambda captures [PR84471] Jason Merrill
2022-12-08 18:30 ` Jason Merrill
2022-12-19 16:00 ` [PATCH PING 2 (tree)] " Jason Merrill
@ 2022-12-20 12:07 ` Richard Biener
2022-12-20 17:38 ` Jason Merrill
2 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2022-12-20 12:07 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
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.
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<tree_decl_map> ();
> h->base.from = from;
> h->to = to;
>
> base-commit: 70596a0fb2a2ec072e1e97e37616e05041dfa4e5
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFA(tree)] c++: source position of lambda captures [PR84471]
2022-12-20 12:07 ` [PATCH RFA(tree)] " Richard Biener
@ 2022-12-20 17:38 ` Jason Merrill
2022-12-20 19:39 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2022-12-20 17:38 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
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?
> /* 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFA(tree)] c++: source position of lambda captures [PR84471]
2022-12-20 17:38 ` Jason Merrill
@ 2022-12-20 19:39 ` Richard Biener
2022-12-21 2:14 ` Jason Merrill
0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2022-12-20 19:39 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
> Am 20.12.2022 um 18:38 schrieb Jason Merrill <jason@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.
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFA(tree)] c++: source position of lambda captures [PR84471]
2022-12-20 19:39 ` Richard Biener
@ 2022-12-21 2:14 ` Jason Merrill
0 siblings, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2022-12-21 2:14 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 8372 bytes --]
On 12/20/22 14:39, Richard Biener wrote:
>
>
>> Am 20.12.2022 um 18:38 schrieb Jason Merrill <jason@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, here's what I pushed.
[-- Attachment #2: 0001-c-source-position-of-lambda-captures-PR84471.patch --]
[-- Type: text/x-patch, Size: 7963 bytes --]
From 302485a70a33f3a86e85ad9051de2b51c5dc0bc0 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Thu, 1 Dec 2022 22:58:28 -0500
Subject: [PATCH] c++: source position of lambda captures [PR84471]
To: gcc-patches@gcc.gnu.org
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 they
avoid the need to unshare to remove the location.
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:
* fold-const.cc (protected_set_expr_location_unshare): Not static.
* tree.h: Declare it.
* tree.cc (decl_value_expr_insert): Use it.
include/ChangeLog:
* ansidecl.h (ATTRIBUTE_WARN_UNUSED_RESULT): Add __.
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/tree.h | 2 ++
include/ansidecl.h | 2 +-
gcc/cp/coroutines.cc | 4 ++--
gcc/cp/decl.cc | 12 +++-------
gcc/fold-const.cc | 2 +-
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 +++
9 files changed, 56 insertions(+), 15 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/tree.h b/gcc/tree.h
index 31d0dcaf249..64a241f51e2 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1289,6 +1289,8 @@ get_expr_source_range (tree expr)
extern void protected_set_expr_location (tree, location_t);
extern void protected_set_expr_location_if_unset (tree, location_t);
+ATTRIBUTE_WARN_UNUSED_RESULT
+extern tree protected_set_expr_location_unshare (tree, location_t);
WARN_UNUSED_RESULT extern tree maybe_wrap_with_location (tree, location_t);
diff --git a/include/ansidecl.h b/include/ansidecl.h
index 056a03ebb6e..4da8069f171 100644
--- a/include/ansidecl.h
+++ b/include/ansidecl.h
@@ -279,7 +279,7 @@ So instead we use the macro below and test it against specific values. */
/* Attribute `warn_unused_result' was valid as of gcc 3.3. */
#ifndef ATTRIBUTE_WARN_UNUSED_RESULT
# if GCC_VERSION >= 3003
-# define ATTRIBUTE_WARN_UNUSED_RESULT __attribute__ ((warn_unused_result))
+# define ATTRIBUTE_WARN_UNUSED_RESULT __attribute__ ((__warn_unused_result__))
# else
# define ATTRIBUTE_WARN_UNUSED_RESULT
# endif
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 88d6c30d8b1..77e2a90f0f9 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 df74d886b28..3c0355a1c39 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -9137,9 +9137,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;
}
@@ -9158,9 +9156,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;
}
@@ -9184,9 +9180,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/fold-const.cc b/gcc/fold-const.cc
index e4c43fbe8cb..42547f433ed 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -164,7 +164,7 @@ expr_location_or (tree t, location_t loc)
/* Similar to protected_set_expr_location, but never modify x in place,
if location can and needs to be set, unshare it. */
-static inline tree
+tree
protected_set_expr_location_unshare (tree x, location_t loc)
{
if (CAN_HAVE_LOCATION_P (x)
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 92199bb6503..581d4489438 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. */
+ to = protected_set_expr_location_unshare (to, UNKNOWN_LOCATION);
+
h = ggc_alloc<tree_decl_map> ();
h->base.from = from;
h->to = to;
--
2.31.1
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-21 2:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-02 15:45 [PATCH RFA(tree)] c++: source position of lambda captures [PR84471] Jason Merrill
2022-12-08 18:30 ` Jason Merrill
2022-12-19 16:00 ` [PATCH PING 2 (tree)] " Jason Merrill
2022-12-20 12:07 ` [PATCH RFA(tree)] " Richard Biener
2022-12-20 17:38 ` Jason Merrill
2022-12-20 19:39 ` Richard Biener
2022-12-21 2:14 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).