Re: [PATCH] Support LABEL_DECL in %qD directive.
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
>
Re: [PATCH] Support LABEL_DECL in %qD directive.
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
Re: [PATCH] Support LABEL_DECL in %qD directive.
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
Re: [PATCH] Support LABEL_DECL in %qD directive.
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
Re: [PATCH] Support LABEL_DECL in %qD directive.
On Wed, Apr 21, 2021 at 10:49 AM Martin Liška 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, "", (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 (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 (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
>
Re: [PATCH] Support LABEL_DECL in %qD directive.
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, "", (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 (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 (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
[PATCH] Support LABEL_DECL in %qD directive.
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(-)
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, "", (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 (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 (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
