diff mbox series

[3/3] Add parentheses around DECL_INIT for .original [PR23872]

Message ID 20240502213918.2029860-3-quic_apinski@quicinc.com
State New
Headers
Series [1/3] Fix printing COMPOUND_EXPR in .original [PR23872] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed

Commit Message

Andrew Pinski May 2, 2024, 9:39 p.m. UTC 
  When we have :
`void f (int y, int z) { int x = ( z++,y); }`

This would have printed the decl's initializer without
parentheses which can confusion if you think that is defining
another variable rather than the compound expression.

This adds parenthese around DECL_INIT if it was a COMPOUND_EXPR.

Bootstrapped and tested on x86_64-linux-gnu.

gcc/ChangeLog:

	* tree-pretty-print.cc (print_declaration): Add parenthese
	around DECL_INIT if it was a COMPOUND_EXPR.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/tree-pretty-print.cc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Richard Biener May 3, 2024, 11:40 a.m. UTC | #1 
On Thu, May 2, 2024 at 11:40 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> When we have :
> `void f (int y, int z) { int x = ( z++,y); }`
>
> This would have printed the decl's initializer without
> parentheses which can confusion if you think that is defining
> another variable rather than the compound expression.
>
> This adds parenthese around DECL_INIT if it was a COMPOUND_EXPR.

Looking it seems we'd hit a similar issue for

 foo ((z++,y), 2);

thus in CALL_EXPR context.  Also

int k;
void foo (int i, int j)
{
  k = (i, 2) + j;
}

dumps as

{
  k = i, j + 2;;
}

(ok that's folded to (i, j + 2) but still).

So shouldn't we bite the bullet and wrap all COMPOUND_EXPRs in
parens instead?  Possibly "tail-calling" the case of
a, b, c in COMPOUND_EXPR dumping itself?

Thanks,
Richard.

> Bootstrapped and tested on x86_64-linux-gnu.
>
> gcc/ChangeLog:
>
>         * tree-pretty-print.cc (print_declaration): Add parenthese
>         around DECL_INIT if it was a COMPOUND_EXPR.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/tree-pretty-print.cc | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> index 825ba74443b..8b766dcd2b8 100644
> --- a/gcc/tree-pretty-print.cc
> +++ b/gcc/tree-pretty-print.cc
> @@ -4240,7 +4240,14 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags, bool
>           pp_equal (pp);
>           pp_space (pp);
>           if (!(flags & TDF_SLIM))
> -           dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
> +           {
> +             bool need_paren = TREE_CODE (DECL_INITIAL (t)) == COMPOUND_EXPR;
> +             if (need_paren)
> +               pp_left_paren (pp);
> +             dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
> +             if (need_paren)
> +               pp_right_paren (pp);
> +           }
>           else
>             pp_string (pp, "<<< omitted >>>");
>         }
> --
> 2.43.0
>
Andrew Pinski May 3, 2024, 10:12 p.m. UTC | #2 
On Fri, May 3, 2024 at 4:41 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Thu, May 2, 2024 at 11:40 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
> >
> > When we have :
> > `void f (int y, int z) { int x = ( z++,y); }`
> >
> > This would have printed the decl's initializer without
> > parentheses which can confusion if you think that is defining
> > another variable rather than the compound expression.
> >
> > This adds parenthese around DECL_INIT if it was a COMPOUND_EXPR.
>
> Looking it seems we'd hit a similar issue for
>
>  foo ((z++,y), 2);
>
> thus in CALL_EXPR context.  Also
>
> int k;
> void foo (int i, int j)
> {
>   k = (i, 2) + j;
> }
>
> dumps as
>
> {
>   k = i, j + 2;;
> }
>
> (ok that's folded to (i, j + 2) but still).
>
> So shouldn't we bite the bullet and wrap all COMPOUND_EXPRs in
> parens instead?  Possibly "tail-calling" the case of
> a, b, c in COMPOUND_EXPR dumping itself?

Let me look into that but it won't be until June due to other things going on.

Thanks,
Andrew

>
> Thanks,
> Richard.
>
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> >         * tree-pretty-print.cc (print_declaration): Add parenthese
> >         around DECL_INIT if it was a COMPOUND_EXPR.
> >
> > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> > ---
> >  gcc/tree-pretty-print.cc | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> > index 825ba74443b..8b766dcd2b8 100644
> > --- a/gcc/tree-pretty-print.cc
> > +++ b/gcc/tree-pretty-print.cc
> > @@ -4240,7 +4240,14 @@ print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags, bool
> >           pp_equal (pp);
> >           pp_space (pp);
> >           if (!(flags & TDF_SLIM))
> > -           dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
> > +           {
> > +             bool need_paren = TREE_CODE (DECL_INITIAL (t)) == COMPOUND_EXPR;
> > +             if (need_paren)
> > +               pp_left_paren (pp);
> > +             dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
> > +             if (need_paren)
> > +               pp_right_paren (pp);
> > +           }
> >           else
> >             pp_string (pp, "<<< omitted >>>");
> >         }
> > --
> > 2.43.0
> >
diff mbox series

Patch

diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 825ba74443b..8b766dcd2b8 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -4240,7 +4240,14 @@  print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags, bool
 	  pp_equal (pp);
 	  pp_space (pp);
 	  if (!(flags & TDF_SLIM))
-	    dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
+	    {
+	      bool need_paren = TREE_CODE (DECL_INITIAL (t)) == COMPOUND_EXPR;
+	      if (need_paren)
+		pp_left_paren (pp);
+	      dump_generic_node (pp, DECL_INITIAL (t), spc, flags, false);
+	      if (need_paren)
+		pp_right_paren (pp);
+	    }
 	  else
 	    pp_string (pp, "<<< omitted >>>");
 	}