Support LABEL_DECL in %qD directive.

Message ID a4187cfc-07dc-dc38-f574-f8249eb79993@suse.cz
State New
Headers show
Series
  • Support LABEL_DECL in %qD directive.
Related show

Commit Message

Martin Liška April 21, 2021, 7:56 a.m.
Hey.

The patch is a refactoring and simplification in tree-cfg.c.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
I verified that the error messages are correctly printed.

Ready to be installed?
Thanks,
Martin

gcc/cp/ChangeLog:

	* error.c (dump_decl): Support anonymous labels.

gcc/ChangeLog:

	* tree-cfg.c (gimple_verify_flow_info): Use qD instead
	of print_generic_expr.
---
 gcc/cp/error.c |  5 ++++-
 gcc/tree-cfg.c | 29 ++++++++++-------------------
 2 files changed, 14 insertions(+), 20 deletions(-)

-- 
2.31.1

Comments

H.J. Lu via Gcc-patches April 21, 2021, 9:04 a.m. | #1
On Wed, Apr 21, 2021 at 09:56:23AM +0200, Martin Liška wrote:
> The patch is a refactoring and simplification in tree-cfg.c.

> 

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

> I verified that the error messages are correctly printed.

> 

> Ready to be installed?

> Thanks,

> Martin

> 

> gcc/cp/ChangeLog:

> 

> 	* error.c (dump_decl): Support anonymous labels.

> 

> gcc/ChangeLog:

> 

> 	* tree-cfg.c (gimple_verify_flow_info): Use qD instead

> 	of print_generic_expr.

> ---

>  gcc/cp/error.c |  5 ++++-

>  gcc/tree-cfg.c | 29 ++++++++++-------------------

>  2 files changed, 14 insertions(+), 20 deletions(-)

> 

> diff --git a/gcc/cp/error.c b/gcc/cp/error.c

> index ff4ae6f4b23..9257a9fce10 100644

> --- a/gcc/cp/error.c

> +++ b/gcc/cp/error.c

> @@ -1362,7 +1362,10 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)

>        break;

>  

>      case LABEL_DECL:

> -      pp_cxx_tree_identifier (pp, DECL_NAME (t));

> +      if (DECL_NAME (t))

> +	pp_cxx_tree_identifier (pp, DECL_NAME (t));

> +      else

> +	pp_printf (pp, "<L%d>", (int) LABEL_DECL_UID (t));


Wouldn't it be better to be consistent with tree-pretty-print.c on this
or perhaps just call dump_generic_node or whatever is used to dump
those e.g. for C?
I see tree-pretty-print.c has much larger code, something for
LABEL_DECL_UID != -1 (one case for flags & TDF_GIMPLE and another without
that), and then 3 different ways for other labels.

> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c

> index 4f63aa69ba8..f985867ced3 100644

> --- a/gcc/tree-cfg.c

> +++ b/gcc/tree-cfg.c

> @@ -5564,37 +5564,29 @@ gimple_verify_flow_info (void)

>  	  label = gimple_label_label (as_a <glabel *> (stmt));

>  	  if (prev_stmt && DECL_NONLOCAL (label))

>  	    {

> -	      error ("nonlocal label ");

> -	      print_generic_expr (stderr, label);

> -	      fprintf (stderr, " is not first in a sequence of labels in bb %d",

> -		       bb->index);

> +	      error ("nonlocal label %qD is not first in a sequence "

> +		     "of labels in bb %d", label, bb->index);

>  	      err = 1;

>  	    }

>  

>  	  if (prev_stmt && EH_LANDING_PAD_NR (label) != 0)

>  	    {

> -	      error ("EH landing pad label ");

> -	      print_generic_expr (stderr, label);

> -	      fprintf (stderr, " is not first in a sequence of labels in bb %d",

> -		       bb->index);

> +	      error ("EH landing pad label %qD is not first in a sequence "

> +		     "of labels in bb %d", label, bb->index);

>  	      err = 1;

>  	    }

>  

>  	  if (label_to_block (cfun, label) != bb)

>  	    {

> -	      error ("label ");

> -	      print_generic_expr (stderr, label);

> -	      fprintf (stderr, " to block does not match in bb %d",

> -		       bb->index);

> +	      error ("label %qD to block does not match in bb %d",

> +		     label, bb->index);

>  	      err = 1;

>  	    }

>  

>  	  if (decl_function_context (label) != current_function_decl)

>  	    {

> -	      error ("label ");

> -	      print_generic_expr (stderr, label);

> -	      fprintf (stderr, " has incorrect context in bb %d",

> -		       bb->index);

> +	      error ("label %qD has incorrect context in bb %d",

> +		     label, bb->index);

>  	      err = 1;

>  	    }

>  	}

> @@ -5616,9 +5608,8 @@ gimple_verify_flow_info (void)

>  

>  	  if (glabel *label_stmt = dyn_cast <glabel *> (stmt))

>  	    {

> -	      error ("label ");

> -	      print_generic_expr (stderr, gimple_label_label (label_stmt));

> -	      fprintf (stderr, " in the middle of basic block %d", bb->index);

> +	      error ("label %qD in the middle of basic block %d",

> +		     gimple_label_label (label_stmt), bb->index);

>  	      err = 1;

>  	    }

>  	}


This LGTM.

	Jakub
H.J. Lu via Gcc-patches April 21, 2021, 10:03 a.m. | #2
On Wed, Apr 21, 2021 at 10:49 AM Martin Liška <mliska@suse.cz> wrote:
>

> Hey.

>

> The patch is a refactoring and simplification in tree-cfg.c.

>

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

> I verified that the error messages are correctly printed.

>

> Ready to be installed?


OK.
Richard.

> Thanks,

> Martin

>

> gcc/cp/ChangeLog:

>

>         * error.c (dump_decl): Support anonymous labels.

>

> gcc/ChangeLog:

>

>         * tree-cfg.c (gimple_verify_flow_info): Use qD instead

>         of print_generic_expr.

> ---

>  gcc/cp/error.c |  5 ++++-

>  gcc/tree-cfg.c | 29 ++++++++++-------------------

>  2 files changed, 14 insertions(+), 20 deletions(-)

>

> diff --git a/gcc/cp/error.c b/gcc/cp/error.c

> index ff4ae6f4b23..9257a9fce10 100644

> --- a/gcc/cp/error.c

> +++ b/gcc/cp/error.c

> @@ -1362,7 +1362,10 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)

>        break;

>

>      case LABEL_DECL:

> -      pp_cxx_tree_identifier (pp, DECL_NAME (t));

> +      if (DECL_NAME (t))

> +       pp_cxx_tree_identifier (pp, DECL_NAME (t));

> +      else

> +       pp_printf (pp, "<L%d>", (int) LABEL_DECL_UID (t));

>        break;

>

>      case CONST_DECL:

> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c

> index 4f63aa69ba8..f985867ced3 100644

> --- a/gcc/tree-cfg.c

> +++ b/gcc/tree-cfg.c

> @@ -5564,37 +5564,29 @@ gimple_verify_flow_info (void)

>           label = gimple_label_label (as_a <glabel *> (stmt));

>           if (prev_stmt && DECL_NONLOCAL (label))

>             {

> -             error ("nonlocal label ");

> -             print_generic_expr (stderr, label);

> -             fprintf (stderr, " is not first in a sequence of labels in bb %d",

> -                      bb->index);

> +             error ("nonlocal label %qD is not first in a sequence "

> +                    "of labels in bb %d", label, bb->index);

>               err = 1;

>             }

>

>           if (prev_stmt && EH_LANDING_PAD_NR (label) != 0)

>             {

> -             error ("EH landing pad label ");

> -             print_generic_expr (stderr, label);

> -             fprintf (stderr, " is not first in a sequence of labels in bb %d",

> -                      bb->index);

> +             error ("EH landing pad label %qD is not first in a sequence "

> +                    "of labels in bb %d", label, bb->index);

>               err = 1;

>             }

>

>           if (label_to_block (cfun, label) != bb)

>             {

> -             error ("label ");

> -             print_generic_expr (stderr, label);

> -             fprintf (stderr, " to block does not match in bb %d",

> -                      bb->index);

> +             error ("label %qD to block does not match in bb %d",

> +                    label, bb->index);

>               err = 1;

>             }

>

>           if (decl_function_context (label) != current_function_decl)

>             {

> -             error ("label ");

> -             print_generic_expr (stderr, label);

> -             fprintf (stderr, " has incorrect context in bb %d",

> -                      bb->index);

> +             error ("label %qD has incorrect context in bb %d",

> +                    label, bb->index);

>               err = 1;

>             }

>         }

> @@ -5616,9 +5608,8 @@ gimple_verify_flow_info (void)

>

>           if (glabel *label_stmt = dyn_cast <glabel *> (stmt))

>             {

> -             error ("label ");

> -             print_generic_expr (stderr, gimple_label_label (label_stmt));

> -             fprintf (stderr, " in the middle of basic block %d", bb->index);

> +             error ("label %qD in the middle of basic block %d",

> +                    gimple_label_label (label_stmt), bb->index);

>               err = 1;

>             }

>         }

> --

> 2.31.1

>
Martin Liška April 21, 2021, 10:52 a.m. | #3
On 4/21/21 11:04 AM, Jakub Jelinek wrote:
> Wouldn't it be better to be consistent with tree-pretty-print.c on this

> or perhaps just call dump_generic_node or whatever is used to dump

> those e.g. for C?


Yes, I'm going to install patch that does:

+      if (DECL_NAME (t))

+       pp_cxx_tree_identifier (pp, DECL_NAME (t));

+      else

+       dump_generic_node (pp, t, 0, TDF_SLIM, false);


Martin
H.J. Lu via Gcc-patches April 21, 2021, 10:56 a.m. | #4
On Wed, Apr 21, 2021 at 12:52:42PM +0200, Martin Liška wrote:
> On 4/21/21 11:04 AM, Jakub Jelinek wrote:

> > Wouldn't it be better to be consistent with tree-pretty-print.c on this

> > or perhaps just call dump_generic_node or whatever is used to dump

> > those e.g. for C?

> 

> Yes, I'm going to install patch that does:

> 

> +      if (DECL_NAME (t))

> 

> +       pp_cxx_tree_identifier (pp, DECL_NAME (t));

> 

> +      else

> 

> +       dump_generic_node (pp, t, 0, TDF_SLIM, false);


Wouldn't flags | TDF_SLIM be better, so that it honors nouid etc.?

	Jakub
Martin Liška April 21, 2021, 11:26 a.m. | #5
On 4/21/21 12:56 PM, Jakub Jelinek wrote:
> On Wed, Apr 21, 2021 at 12:52:42PM +0200, Martin Liška wrote:

>> On 4/21/21 11:04 AM, Jakub Jelinek wrote:

>>> Wouldn't it be better to be consistent with tree-pretty-print.c on this

>>> or perhaps just call dump_generic_node or whatever is used to dump

>>> those e.g. for C?

>>

>> Yes, I'm going to install patch that does:

>>

>> +      if (DECL_NAME (t))

>>

>> +       pp_cxx_tree_identifier (pp, DECL_NAME (t));

>>

>> +      else

>>

>> +       dump_generic_node (pp, t, 0, TDF_SLIM, false);

> 

> Wouldn't flags | TDF_SLIM be better, so that it honors nouid etc.?

> 

> 	Jakub

> 


Good point, fixed that.

Thanks,
Martin
Martin Liška April 21, 2021, 11:32 a.m. | #6
On 4/21/21 1:26 PM, Martin Liška wrote:
> On 4/21/21 12:56 PM, Jakub Jelinek wrote:

>> On Wed, Apr 21, 2021 at 12:52:42PM +0200, Martin Liška wrote:

>>> On 4/21/21 11:04 AM, Jakub Jelinek wrote:

>>>> Wouldn't it be better to be consistent with tree-pretty-print.c on this

>>>> or perhaps just call dump_generic_node or whatever is used to dump

>>>> those e.g. for C?

>>>

>>> Yes, I'm going to install patch that does:

>>>

>>> +      if (DECL_NAME (t))

>>>

>>> +       pp_cxx_tree_identifier (pp, DECL_NAME (t));

>>>

>>> +      else

>>>

>>> +       dump_generic_node (pp, t, 0, TDF_SLIM, false);

>>

>> Wouldn't flags | TDF_SLIM be better, so that it honors nouid etc.?

>>

>> 	Jakub

>>

> 

> Good point, fixed that.


Oh, it leads to:

/home/marxin/Programming/gcc/gcc/cp/error.c: In function ‘void dump_decl(cxx_pretty_printer*, tree, int)’:

/home/marxin/Programming/gcc/gcc/cp/error.c:1368:37: error: invalid conversion from ‘int’ to ‘dump_flags_t’ {aka ‘dump_flag’} [-fpermissive]

 1368 |  dump_generic_node (pp, t, 0, flags | TDF_SLIM, false);

      |                               ~~~~~~^~~~~~~~~~

      |                                     |

      |                                     int


Thus I reverted the change. It uses flags defined in cp/cp-tree.h:
#define TFF_PLAIN_IDENTIFIER			(0)
#define TFF_SCOPE				(1)
#define TFF_CHASE_TYPEDEF			(1 << 1)
#define TFF_DECL_SPECIFIERS			(1 << 2)
...

Martin

> 

> Thanks,

> Martin

>

Patch

diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index ff4ae6f4b23..9257a9fce10 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -1362,7 +1362,10 @@  dump_decl (cxx_pretty_printer *pp, tree t, int flags)
       break;
 
     case LABEL_DECL:
-      pp_cxx_tree_identifier (pp, DECL_NAME (t));
+      if (DECL_NAME (t))
+	pp_cxx_tree_identifier (pp, DECL_NAME (t));
+      else
+	pp_printf (pp, "<L%d>", (int) LABEL_DECL_UID (t));
       break;
 
     case CONST_DECL:
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 4f63aa69ba8..f985867ced3 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -5564,37 +5564,29 @@  gimple_verify_flow_info (void)
 	  label = gimple_label_label (as_a <glabel *> (stmt));
 	  if (prev_stmt && DECL_NONLOCAL (label))
 	    {
-	      error ("nonlocal label ");
-	      print_generic_expr (stderr, label);
-	      fprintf (stderr, " is not first in a sequence of labels in bb %d",
-		       bb->index);
+	      error ("nonlocal label %qD is not first in a sequence "
+		     "of labels in bb %d", label, bb->index);
 	      err = 1;
 	    }
 
 	  if (prev_stmt && EH_LANDING_PAD_NR (label) != 0)
 	    {
-	      error ("EH landing pad label ");
-	      print_generic_expr (stderr, label);
-	      fprintf (stderr, " is not first in a sequence of labels in bb %d",
-		       bb->index);
+	      error ("EH landing pad label %qD is not first in a sequence "
+		     "of labels in bb %d", label, bb->index);
 	      err = 1;
 	    }
 
 	  if (label_to_block (cfun, label) != bb)
 	    {
-	      error ("label ");
-	      print_generic_expr (stderr, label);
-	      fprintf (stderr, " to block does not match in bb %d",
-		       bb->index);
+	      error ("label %qD to block does not match in bb %d",
+		     label, bb->index);
 	      err = 1;
 	    }
 
 	  if (decl_function_context (label) != current_function_decl)
 	    {
-	      error ("label ");
-	      print_generic_expr (stderr, label);
-	      fprintf (stderr, " has incorrect context in bb %d",
-		       bb->index);
+	      error ("label %qD has incorrect context in bb %d",
+		     label, bb->index);
 	      err = 1;
 	    }
 	}
@@ -5616,9 +5608,8 @@  gimple_verify_flow_info (void)
 
 	  if (glabel *label_stmt = dyn_cast <glabel *> (stmt))
 	    {
-	      error ("label ");
-	      print_generic_expr (stderr, gimple_label_label (label_stmt));
-	      fprintf (stderr, " in the middle of basic block %d", bb->index);
+	      error ("label %qD in the middle of basic block %d",
+		     gimple_label_label (label_stmt), bb->index);
 	      err = 1;
 	    }
 	}