Re: [PATCH] c++: set loc on call even if result is discarded
On 4/7/22 18:48, Alexandre Oliva wrote:
On Apr 6, 2022, Jason Merrill wrote:
On 4/6/22 15:37, Alexandre Oliva wrote:
Need to adjust this subject line, as well.
*nod*, thanks
* tree.cc (protected_set_expr_location): Propagate locus to
call wrapped in cast-to-void.
I'm reluctant to put this C++-specific change in a simple function
shared by all languages;
Perhaps it benefits other languages as well? The effect is presumably
desirable on other languages too: setting a cast-to-void's location
seems completely ineffective, as it's eventually thrown away, and
perhaps propagating the location to any operand (rather than just calls)
would carry out the intent of [protected_]set_expr_location more
effectively. It doesn't feel right to require every caller to worry
about that.
how about handling it in set_cleanup_locs instead?
Like this? That seems reasonable to me. I'll give it a spin.
Yes, or perhaps STRIP_NOPS and set the location on whatever is left. OK
either way.
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index a7f6449dafd2e..43627ed30afcb 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -609,7 +609,17 @@ set_cleanup_locs (tree stmts, location_t loc)
{
if (TREE_CODE (stmts) == CLEANUP_STMT)
{
- protected_set_expr_location (CLEANUP_EXPR (stmts), loc);
+ tree t = CLEANUP_EXPR (stmts);
+ protected_set_expr_location (t, loc);
+ /* Avoid locus differences for C++ cdtor calls depending on whether
+cdtor_returns_this: a conversion to void is added to discard the return
+value, and this conversion ends up carrying the location, and when it
+gets discarded, the location is lost. So hold it in the call as
+well. */
+ if (TREE_CODE (t) == NOP_EXPR
+ && TREE_TYPE (t) == void_type_node
+ && TREE_CODE (TREE_OPERAND (t, 0)) == CALL_EXPR)
+ protected_set_expr_location (TREE_OPERAND (t, 0), loc);
set_cleanup_locs (CLEANUP_BODY (stmts), loc);
}
else if (TREE_CODE (stmts) == STATEMENT_LIST)
Re: [PATCH] c++: set loc on call even if result is discarded
On Fri, Apr 8, 2022 at 12:49 AM Alexandre Oliva via Gcc-patches
wrote:
>
> On Apr 6, 2022, Jason Merrill wrote:
>
> > On 4/6/22 15:37, Alexandre Oliva wrote:
> > Need to adjust this subject line, as well.
>
> *nod*, thanks
>
> >> * tree.cc (protected_set_expr_location): Propagate locus to
> >> call wrapped in cast-to-void.
>
> > I'm reluctant to put this C++-specific change in a simple function
> > shared by all languages;
>
> Perhaps it benefits other languages as well? The effect is presumably
> desirable on other languages too: setting a cast-to-void's location
> seems completely ineffective, as it's eventually thrown away, and
> perhaps propagating the location to any operand (rather than just calls)
> would carry out the intent of [protected_]set_expr_location more
> effectively. It doesn't feel right to require every caller to worry
> about that.
Hmm, but then maybe it would be better the task of the code
actually throwing away the cast? I agree the original
proposed location to fix this looks bad.
> > how about handling it in set_cleanup_locs instead?
>
> Like this? That seems reasonable to me. I'll give it a spin.
>
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index a7f6449dafd2e..43627ed30afcb 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -609,7 +609,17 @@ set_cleanup_locs (tree stmts, location_t loc)
> {
>if (TREE_CODE (stmts) == CLEANUP_STMT)
> {
> - protected_set_expr_location (CLEANUP_EXPR (stmts), loc);
> + tree t = CLEANUP_EXPR (stmts);
> + protected_set_expr_location (t, loc);
> + /* Avoid locus differences for C++ cdtor calls depending on whether
> +cdtor_returns_this: a conversion to void is added to discard the
> return
> +value, and this conversion ends up carrying the location, and when it
> +gets discarded, the location is lost. So hold it in the call as
> +well. */
> + if (TREE_CODE (t) == NOP_EXPR
> + && TREE_TYPE (t) == void_type_node
> + && TREE_CODE (TREE_OPERAND (t, 0)) == CALL_EXPR)
> + protected_set_expr_location (TREE_OPERAND (t, 0), loc);
>set_cleanup_locs (CLEANUP_BODY (stmts), loc);
> }
>else if (TREE_CODE (stmts) == STATEMENT_LIST)
>
>
> --
> Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
>Free Software Activist GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts. Ask me about
Re: [PATCH] c++: set loc on call even if result is discarded
On Apr 6, 2022, Jason Merrill wrote:
> On 4/6/22 15:37, Alexandre Oliva wrote:
> Need to adjust this subject line, as well.
*nod*, thanks
>> * tree.cc (protected_set_expr_location): Propagate locus to
>> call wrapped in cast-to-void.
> I'm reluctant to put this C++-specific change in a simple function
> shared by all languages;
Perhaps it benefits other languages as well? The effect is presumably
desirable on other languages too: setting a cast-to-void's location
seems completely ineffective, as it's eventually thrown away, and
perhaps propagating the location to any operand (rather than just calls)
would carry out the intent of [protected_]set_expr_location more
effectively. It doesn't feel right to require every caller to worry
about that.
> how about handling it in set_cleanup_locs instead?
Like this? That seems reasonable to me. I'll give it a spin.
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index a7f6449dafd2e..43627ed30afcb 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -609,7 +609,17 @@ set_cleanup_locs (tree stmts, location_t loc)
{
if (TREE_CODE (stmts) == CLEANUP_STMT)
{
- protected_set_expr_location (CLEANUP_EXPR (stmts), loc);
+ tree t = CLEANUP_EXPR (stmts);
+ protected_set_expr_location (t, loc);
+ /* Avoid locus differences for C++ cdtor calls depending on whether
+cdtor_returns_this: a conversion to void is added to discard the return
+value, and this conversion ends up carrying the location, and when it
+gets discarded, the location is lost. So hold it in the call as
+well. */
+ if (TREE_CODE (t) == NOP_EXPR
+ && TREE_TYPE (t) == void_type_node
+ && TREE_CODE (TREE_OPERAND (t, 0)) == CALL_EXPR)
+ protected_set_expr_location (TREE_OPERAND (t, 0), loc);
set_cleanup_locs (CLEANUP_BODY (stmts), loc);
}
else if (TREE_CODE (stmts) == STATEMENT_LIST)
--
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts. Ask me about
