Re: [PATCH 2/3] Improve DECL_EXPR printing in .original [PR23872]
On Fri, May 3, 2024 at 4:36 AM Richard Biener
wrote:
>
> On Thu, May 2, 2024 at 11:40 PM Andrew Pinski
> wrote:
> >
> > Right now we don't print that a DECL_EXPR and we get
> > basically double output of the decls and it looks confusing.
> > This fixes that.
> > for the simple example:
> > `void foo () { int result = 0;}`
> > This gives:
> > ```
> > {
> > int result = 0;
> >
> > DECL_EXPR;
> > }
> > ```
>
> Hmm, I think it would be better if it were
>
> {
>
> int result = 0;
> }
>
> so omit the dumping from the BLOCK_VARS(?) when the variable has a DECL_EXPR.
> That more easily lets us spot use-before-DECL_EXPR issues.
Yes, yes that would be definitely better. Let me see if I can figure
that out. I might not be able to get back to this until June though.
>
> So I don't think this patch is an improvement?
Ok, yes I agree.
Thanks,
Andrew
>
> Thanks,
> Richard.
>
> > Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > gcc/ChangeLog:
> >
> > * tree-pretty-print.cc (dump_generic_node ): Print
> > out `DECL_EXPR<...>` around the decl and update the call to
> > print_declaration to pass false for new argument and pass 0
> > for the spacing.
> > (print_declaration): Add argument is_stmt and don't print
> > a semicolon nor the initializer.
> > * tree-pretty-print.h (print_declaration): Add bool argument
> > and default it to true.
> >
> > Signed-off-by: Andrew Pinski
> > ---
> > gcc/tree-pretty-print.cc | 18 +++---
> > gcc/tree-pretty-print.h | 2 +-
> > 2 files changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> > index f9ad8562078..825ba74443b 100644
> > --- a/gcc/tree-pretty-print.cc
> > +++ b/gcc/tree-pretty-print.cc
> > @@ -2917,8 +2917,9 @@ dump_generic_node (pretty_printer *pp, tree node, int
> > spc, dump_flags_t flags,
> >break;
> >
> > case DECL_EXPR:
> > - print_declaration (pp, DECL_EXPR_DECL (node), spc, flags);
> > - is_stmt = false;
> > + pp_string (pp, "DECL_EXPR<");
> > + print_declaration (pp, DECL_EXPR_DECL (node), 0, flags, false);
> > + pp_greater (pp);
> >break;
> >
> > case COND_EXPR:
> > @@ -4151,10 +4152,11 @@ dump_generic_node (pretty_printer *pp, tree node,
> > int spc, dump_flags_t flags,
> >return spc;
> > }
> >
> > -/* Print the declaration of a variable. */
> > +/* Print the declaration of a variable, T to PP starting with SPC spaces
> > with FLAGS
> > + and called as IS_STMT a statement or not. */
> >
> > void
> > -print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
> > +print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t
> > flags, bool is_stmt)
> > {
> >INDENT (spc);
> >
> > @@ -4162,7 +4164,8 @@ print_declaration (pretty_printer *pp, tree t, int
> > spc, dump_flags_t flags)
> > {
> >pp_string(pp, "namelist ");
> >dump_decl_name (pp, t, flags);
> > - pp_semicolon (pp);
> > + if (is_stmt)
> > + pp_semicolon (pp);
> >return;
> > }
> >
> > @@ -4231,7 +4234,7 @@ print_declaration (pretty_printer *pp, tree t, int
> > spc, dump_flags_t flags)
> >if (TREE_CODE (t) != FUNCTION_DECL)
> > {
> >/* Print the initial value. */
> > - if (DECL_INITIAL (t))
> > + if (DECL_INITIAL (t) && is_stmt)
> > {
> > pp_space (pp);
> > pp_equal (pp);
> > @@ -4250,7 +4253,8 @@ print_declaration (pretty_printer *pp, tree t, int
> > spc, dump_flags_t flags)
> >pp_right_bracket (pp);
> > }
> >
> > - pp_semicolon (pp);
> > + if (is_stmt)
> > +pp_semicolon (pp);
> > }
> >
> >
> > diff --git a/gcc/tree-pretty-print.h b/gcc/tree-pretty-print.h
> > index 0da6242629b..660c17410a9 100644
> > --- a/gcc/tree-pretty-print.h
> > +++ b/gcc/tree-pretty-print.h
> > @@ -47,7 +47,7 @@ extern void dump_omp_loop_non_rect_expr (pretty_printer
> > *, tree, int,
> > dump_flags_t);
> > extern void print_omp_context_selector (FILE *, tree, dump_flags_t);
> > extern int dump_generic_node (pretty_printer *, tree, int, dump_flags_t,
> > bool);
> > -extern void print_declaration (pretty_printer *, tree, int, dump_flags_t);
> > +extern void print_declaration (pretty_printer *, tree, int, dump_flags_t,
> > bool = true);
> > extern int op_code_prio (enum tree_code);
> > extern int op_prio (const_tree);
> > extern const char *op_symbol_code (enum tree_code, dump_flags_t =
> > TDF_NONE);
> > --
> > 2.43.0
> >
Re: [PATCH 2/3] Improve DECL_EXPR printing in .original [PR23872]
On Thu, May 2, 2024 at 11:40 PM Andrew Pinski wrote:
>
> Right now we don't print that a DECL_EXPR and we get
> basically double output of the decls and it looks confusing.
> This fixes that.
> for the simple example:
> `void foo () { int result = 0;}`
> This gives:
> ```
> {
> int result = 0;
>
> DECL_EXPR;
> }
> ```
Hmm, I think it would be better if it were
{
int result = 0;
}
so omit the dumping from the BLOCK_VARS(?) when the variable has a DECL_EXPR.
That more easily lets us spot use-before-DECL_EXPR issues.
So I don't think this patch is an improvement?
Thanks,
Richard.
> Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> gcc/ChangeLog:
>
> * tree-pretty-print.cc (dump_generic_node ): Print
> out `DECL_EXPR<...>` around the decl and update the call to
> print_declaration to pass false for new argument and pass 0
> for the spacing.
> (print_declaration): Add argument is_stmt and don't print
> a semicolon nor the initializer.
> * tree-pretty-print.h (print_declaration): Add bool argument
> and default it to true.
>
> Signed-off-by: Andrew Pinski
> ---
> gcc/tree-pretty-print.cc | 18 +++---
> gcc/tree-pretty-print.h | 2 +-
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
> index f9ad8562078..825ba74443b 100644
> --- a/gcc/tree-pretty-print.cc
> +++ b/gcc/tree-pretty-print.cc
> @@ -2917,8 +2917,9 @@ dump_generic_node (pretty_printer *pp, tree node, int
> spc, dump_flags_t flags,
>break;
>
> case DECL_EXPR:
> - print_declaration (pp, DECL_EXPR_DECL (node), spc, flags);
> - is_stmt = false;
> + pp_string (pp, "DECL_EXPR<");
> + print_declaration (pp, DECL_EXPR_DECL (node), 0, flags, false);
> + pp_greater (pp);
>break;
>
> case COND_EXPR:
> @@ -4151,10 +4152,11 @@ dump_generic_node (pretty_printer *pp, tree node, int
> spc, dump_flags_t flags,
>return spc;
> }
>
> -/* Print the declaration of a variable. */
> +/* Print the declaration of a variable, T to PP starting with SPC spaces
> with FLAGS
> + and called as IS_STMT a statement or not. */
>
> void
> -print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
> +print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags,
> bool is_stmt)
> {
>INDENT (spc);
>
> @@ -4162,7 +4164,8 @@ print_declaration (pretty_printer *pp, tree t, int spc,
> dump_flags_t flags)
> {
>pp_string(pp, "namelist ");
>dump_decl_name (pp, t, flags);
> - pp_semicolon (pp);
> + if (is_stmt)
> + pp_semicolon (pp);
>return;
> }
>
> @@ -4231,7 +4234,7 @@ print_declaration (pretty_printer *pp, tree t, int spc,
> dump_flags_t flags)
>if (TREE_CODE (t) != FUNCTION_DECL)
> {
>/* Print the initial value. */
> - if (DECL_INITIAL (t))
> + if (DECL_INITIAL (t) && is_stmt)
> {
> pp_space (pp);
> pp_equal (pp);
> @@ -4250,7 +4253,8 @@ print_declaration (pretty_printer *pp, tree t, int spc,
> dump_flags_t flags)
>pp_right_bracket (pp);
> }
>
> - pp_semicolon (pp);
> + if (is_stmt)
> +pp_semicolon (pp);
> }
>
>
> diff --git a/gcc/tree-pretty-print.h b/gcc/tree-pretty-print.h
> index 0da6242629b..660c17410a9 100644
> --- a/gcc/tree-pretty-print.h
> +++ b/gcc/tree-pretty-print.h
> @@ -47,7 +47,7 @@ extern void dump_omp_loop_non_rect_expr (pretty_printer *,
> tree, int,
> dump_flags_t);
> extern void print_omp_context_selector (FILE *, tree, dump_flags_t);
> extern int dump_generic_node (pretty_printer *, tree, int, dump_flags_t,
> bool);
> -extern void print_declaration (pretty_printer *, tree, int, dump_flags_t);
> +extern void print_declaration (pretty_printer *, tree, int, dump_flags_t,
> bool = true);
> extern int op_code_prio (enum tree_code);
> extern int op_prio (const_tree);
> extern const char *op_symbol_code (enum tree_code, dump_flags_t = TDF_NONE);
> --
> 2.43.0
>
[PATCH 2/3] Improve DECL_EXPR printing in .original [PR23872]
Right now we don't print that a DECL_EXPR and we get
basically double output of the decls and it looks confusing.
This fixes that.
for the simple example:
`void foo () { int result = 0;}`
This gives:
```
{
int result = 0;
DECL_EXPR;
}
```
Bootstrapped and tested on x86_64-linux-gnu with no regressions.
gcc/ChangeLog:
* tree-pretty-print.cc (dump_generic_node ): Print
out `DECL_EXPR<...>` around the decl and update the call to
print_declaration to pass false for new argument and pass 0
for the spacing.
(print_declaration): Add argument is_stmt and don't print
a semicolon nor the initializer.
* tree-pretty-print.h (print_declaration): Add bool argument
and default it to true.
Signed-off-by: Andrew Pinski
---
gcc/tree-pretty-print.cc | 18 +++---
gcc/tree-pretty-print.h | 2 +-
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index f9ad8562078..825ba74443b 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -2917,8 +2917,9 @@ dump_generic_node (pretty_printer *pp, tree node, int
spc, dump_flags_t flags,
break;
case DECL_EXPR:
- print_declaration (pp, DECL_EXPR_DECL (node), spc, flags);
- is_stmt = false;
+ pp_string (pp, "DECL_EXPR<");
+ print_declaration (pp, DECL_EXPR_DECL (node), 0, flags, false);
+ pp_greater (pp);
break;
case COND_EXPR:
@@ -4151,10 +4152,11 @@ dump_generic_node (pretty_printer *pp, tree node, int
spc, dump_flags_t flags,
return spc;
}
-/* Print the declaration of a variable. */
+/* Print the declaration of a variable, T to PP starting with SPC spaces with
FLAGS
+ and called as IS_STMT a statement or not. */
void
-print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags)
+print_declaration (pretty_printer *pp, tree t, int spc, dump_flags_t flags,
bool is_stmt)
{
INDENT (spc);
@@ -4162,7 +4164,8 @@ print_declaration (pretty_printer *pp, tree t, int spc,
dump_flags_t flags)
{
pp_string(pp, "namelist ");
dump_decl_name (pp, t, flags);
- pp_semicolon (pp);
+ if (is_stmt)
+ pp_semicolon (pp);
return;
}
@@ -4231,7 +4234,7 @@ print_declaration (pretty_printer *pp, tree t, int spc,
dump_flags_t flags)
if (TREE_CODE (t) != FUNCTION_DECL)
{
/* Print the initial value. */
- if (DECL_INITIAL (t))
+ if (DECL_INITIAL (t) && is_stmt)
{
pp_space (pp);
pp_equal (pp);
@@ -4250,7 +4253,8 @@ print_declaration (pretty_printer *pp, tree t, int spc,
dump_flags_t flags)
pp_right_bracket (pp);
}
- pp_semicolon (pp);
+ if (is_stmt)
+pp_semicolon (pp);
}
diff --git a/gcc/tree-pretty-print.h b/gcc/tree-pretty-print.h
index 0da6242629b..660c17410a9 100644
--- a/gcc/tree-pretty-print.h
+++ b/gcc/tree-pretty-print.h
@@ -47,7 +47,7 @@ extern void dump_omp_loop_non_rect_expr (pretty_printer *,
tree, int,
dump_flags_t);
extern void print_omp_context_selector (FILE *, tree, dump_flags_t);
extern int dump_generic_node (pretty_printer *, tree, int, dump_flags_t, bool);
-extern void print_declaration (pretty_printer *, tree, int, dump_flags_t);
+extern void print_declaration (pretty_printer *, tree, int, dump_flags_t, bool
= true);
extern int op_code_prio (enum tree_code);
extern int op_prio (const_tree);
extern const char *op_symbol_code (enum tree_code, dump_flags_t = TDF_NONE);
--
2.43.0
