[C++] Fix pretty-print of enumerator ids (PR c++/87364)

Hi,

A PR to fix Bug 87364 - Pretty print of enumerator never prints the id,
always falls back to C-style cast output

The fix is tested with the code attached to the bug report and additional
usage over the past few weeks.

Bootstrap and regtested on x86_64-linux. OK for trunk?


2018-10-08  Will Wray  <wjwray@gmail.com>

PR c++/87364
* c-pretty-print.c (pp_c_enumeration_constant): fix comparison.
* cxx-pretty-print.c (pp_cxx_enumeration_constant): New; add
nested-specifiers prefix to enum id plus enum type for scoped enum.

OK.
On Mon, Oct 8, 2018 at 1:12 PM will wray <wjwray@gmail.com> wrote:
>
> Hi,
>
> A PR to fix Bug 87364 - Pretty print of enumerator never prints the id,
> always falls back to C-style cast output
>
> The fix is tested with the code attached to the bug report and additional
> usage over the past few weeks.
>
> Bootstrap and regtested on x86_64-linux. OK for trunk?
>
>
> 2018-10-08  Will Wray  <wjwray@gmail.com>
>
> PR c++/87364
> * c-pretty-print.c (pp_c_enumeration_constant): fix comparison.
> * cxx-pretty-print.c (pp_cxx_enumeration_constant): New; add
> nested-specifiers prefix to enum id plus enum type for scoped enum.
>
> Index: gcc/c-family/c-pretty-print.h
> ===================================================================
> --- gcc/c-family/c-pretty-print.h (revision 264938)
> +++ gcc/c-family/c-pretty-print.h (working copy)
> @@ -128,11 +128,13 @@ void pp_c_logical_or_expression (c_prett
>  void pp_c_expression_list (c_pretty_printer *, tree);
>  void pp_c_constructor_elts (c_pretty_printer *, vec<constructor_elt,
> va_gc> *);
>  void pp_c_call_argument_list (c_pretty_printer *, tree);
> +void pp_c_type_cast (c_pretty_printer *, tree);
>  void pp_c_cast_expression (c_pretty_printer *, tree);
>  void pp_c_init_declarator (c_pretty_printer *, tree);
>  void pp_c_ws_string (c_pretty_printer *, const char *);
>  void pp_c_identifier (c_pretty_printer *, const char *);
>  void pp_c_string_literal (c_pretty_printer *, tree);
> +void pp_c_integer_constant (c_pretty_printer *, tree);
>
>  void print_c_tree (FILE *file, tree t);
>
> Index: gcc/c-family/c-pretty-print.c
> ===================================================================
> --- gcc/c-family/c-pretty-print.c (revision 264938)
> +++ gcc/c-family/c-pretty-print.c (working copy)
> @@ -192,7 +192,7 @@ pp_c_cv_qualifiers (c_pretty_printer *pp
>
>  /* Pretty-print T using the type-cast notation '( type-name )'.  */
>
> -static void
> +void
>  pp_c_type_cast (c_pretty_printer *pp, tree t)
>  {
>    pp_c_left_paren (pp);
> @@ -908,7 +908,7 @@ pp_c_void_constant (c_pretty_printer *pp
>
>  /* Pretty-print an INTEGER literal.  */
>
> -static void
> +void
>  pp_c_integer_constant (c_pretty_printer *pp, tree i)
>  {
>    if (tree_fits_shwi_p (i))
> @@ -968,21 +968,20 @@ pp_c_bool_constant (c_pretty_printer *pp
>      pp_unsupported_tree (pp, b);
>  }
>
> -/* Attempt to print out an ENUMERATOR.  Return true on success.  Else
> return
> -   false; that means the value was obtained by a cast, in which case
> -   print out the type-id part of the cast-expression -- the casted value
> -   is then printed by pp_c_integer_literal.  */
> +/* Given a value e of ENUMERAL_TYPE:
> +   Print out the first ENUMERATOR id with value e, if one is found,
> +   else print out the value as a C-style cast (type-id)value.  */
>
> -static bool
> +static void
>  pp_c_enumeration_constant (c_pretty_printer *pp, tree e)
>  {
> -  bool value_is_named = true;
>    tree type = TREE_TYPE (e);
>    tree value;
>
>    /* Find the name of this constant.  */
>    for (value = TYPE_VALUES (type);
> -       value != NULL_TREE && !tree_int_cst_equal (TREE_VALUE (value), e);
> +       value != NULL_TREE
> +   && !tree_int_cst_equal (DECL_INITIAL (TREE_VALUE (value)), e);
>         value = TREE_CHAIN (value))
>      ;
>
> @@ -992,10 +991,8 @@ pp_c_enumeration_constant (c_pretty_prin
>      {
>        /* Value must have been cast.  */
>        pp_c_type_cast (pp, type);
> -      value_is_named = false;
> +      pp_c_integer_constant (pp, e);
>      }
> -
> -  return value_is_named;
>  }
>
>  /* Print out a REAL value as a decimal-floating-constant.  */
> @@ -1140,9 +1137,8 @@ c_pretty_printer::constant (tree e)
>     pp_c_bool_constant (this, e);
>   else if (type == char_type_node)
>     pp_c_character_constant (this, e);
> - else if (TREE_CODE (type) == ENUMERAL_TYPE
> - && pp_c_enumeration_constant (this, e))
> -   ;
> + else if (TREE_CODE (type) == ENUMERAL_TYPE)
> +   pp_c_enumeration_constant (this, e);
>   else
>     pp_c_integer_constant (this, e);
>        }
> Index: gcc/cp/cxx-pretty-print.c
> ===================================================================
> --- gcc/cp/cxx-pretty-print.c (revision 264938)
> +++ gcc/cp/cxx-pretty-print.c (working copy)
> @@ -294,6 +294,39 @@ pp_cxx_qualified_id (cxx_pretty_printer
>      }
>  }
>
> +/* Given a value e of ENUMERAL_TYPE:
> +   Print out the first ENUMERATOR id with value e, if one is found,
> +   (including nested names but excluding the enum name if unscoped)
> +   else print out the value as a C-style cast (type-id)value.  */
> +
> +static void
> +pp_cxx_enumeration_constant (cxx_pretty_printer *pp, tree e)
> +{
> +  tree type = TREE_TYPE (e);
> +  tree value;
> +
> +  /* Find the name of this constant.  */
> +  for (value = TYPE_VALUES (type);
> +       value != NULL_TREE
> +   && !tree_int_cst_equal (DECL_INITIAL (TREE_VALUE (value)), e);
> +       value = TREE_CHAIN (value))
> +    ;
> +
> +  if (value != NULL_TREE)
> +    {
> +      if (!ENUM_IS_SCOPED (type))
> + type = get_containing_scope (type);
> +      pp_cxx_nested_name_specifier (pp, type);
> +      pp->id_expression (TREE_PURPOSE (value));
> +    }
> +  else
> +    {
> +      /* Value must have been cast.  */
> +       pp_c_type_cast (pp, type);
> +       pp_c_integer_constant (pp, e);
> +    }
> +}
> +
>
>  void
>  cxx_pretty_printer::constant (tree t)
> @@ -317,6 +350,11 @@ cxx_pretty_printer::constant (tree t)
>     pp_string (this, "nullptr");
>     break;
>   }
> +      else if (TREE_CODE (TREE_TYPE (t)) == ENUMERAL_TYPE)
> + {
> +   pp_cxx_enumeration_constant (this, t);
> +   break;
> + }
>        /* fall through.  */
>
>      default:

On 10/8/18 11:12 AM, will wray wrote:
> Hi,
> 
> A PR to fix Bug 87364 - Pretty print of enumerator never prints the id,
> always falls back to C-style cast output
> 
> The fix is tested with the code attached to the bug report and additional
> usage over the past few weeks.
> 
> Bootstrap and regtested on x86_64-linux. OK for trunk?
> 
> 
> 2018-10-08  Will Wray  <wjwray@gmail.com>
> 
> PR c++/87364
> * c-pretty-print.c (pp_c_enumeration_constant): fix comparison.
> * cxx-pretty-print.c (pp_cxx_enumeration_constant): New; add
> nested-specifiers prefix to enum id plus enum type for scoped enum.
THanks.  I fixed up some whitespace issues and the ChangeLog entry
before committing to the trunk.

Also note for any future contributions, it looks like your mailer is
messing up tabs and wrapping lines. That makes applying the patch more
difficult than it should be.

Jeff

On Fri, 12 Oct 2018 at 05:37, Jeff Law <law@redhat.com> wrote:
>
> On 10/8/18 11:12 AM, will wray wrote:
> > Hi,
> >
> > A PR to fix Bug 87364 - Pretty print of enumerator never prints the id,
> > always falls back to C-style cast output
> >
> > The fix is tested with the code attached to the bug report and additional
> > usage over the past few weeks.
> >
> > Bootstrap and regtested on x86_64-linux. OK for trunk?
> >
> >
> > 2018-10-08  Will Wray  <wjwray@gmail.com>
> >
> > PR c++/87364
> > * c-pretty-print.c (pp_c_enumeration_constant): fix comparison.
> > * cxx-pretty-print.c (pp_cxx_enumeration_constant): New; add
> > nested-specifiers prefix to enum id plus enum type for scoped enum.
> THanks.  I fixed up some whitespace issues and the ChangeLog entry
> before committing to the trunk.
>
> Also note for any future contributions, it looks like your mailer is
> messing up tabs and wrapping lines. That makes applying the patch more
> difficult than it should be.
>

Hi,

This commit broke the GCC builds:
/tmp/3768585_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/c-family/c-pretty-print.c:
In member function 'virtual void c_pretty_printer::constant(tree)':
/tmp/3768585_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/c-family/c-pretty-print.c:1141:39:
error: expected ';' before ')' token
    pp_c_enumeration_constant (this, e))
                                       ^
make[2]: *** [c-family/c-pretty-print.o] Error 1

> Jeff

On Fri, 12 Oct 2018 at 07:47, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
>
> On Fri, 12 Oct 2018 at 05:37, Jeff Law <law@redhat.com> wrote:
> >
> > On 10/8/18 11:12 AM, will wray wrote:
> > > Hi,
> > >
> > > A PR to fix Bug 87364 - Pretty print of enumerator never prints the id,
> > > always falls back to C-style cast output
> > >
> > > The fix is tested with the code attached to the bug report and additional
> > > usage over the past few weeks.
> > >
> > > Bootstrap and regtested on x86_64-linux. OK for trunk?
> > >
> > >
> > > 2018-10-08  Will Wray  <wjwray@gmail.com>
> > >
> > > PR c++/87364
> > > * c-pretty-print.c (pp_c_enumeration_constant): fix comparison.
> > > * cxx-pretty-print.c (pp_cxx_enumeration_constant): New; add
> > > nested-specifiers prefix to enum id plus enum type for scoped enum.
> > THanks.  I fixed up some whitespace issues and the ChangeLog entry
> > before committing to the trunk.
> >
> > Also note for any future contributions, it looks like your mailer is
> > messing up tabs and wrapping lines. That makes applying the patch more
> > difficult than it should be.
> >
>
> Hi,
>
> This commit broke the GCC builds:
> /tmp/3768585_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/c-family/c-pretty-print.c:
> In member function 'virtual void c_pretty_printer::constant(tree)':
> /tmp/3768585_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/c-family/c-pretty-print.c:1141:39:
> error: expected ';' before ')' token
>     pp_c_enumeration_constant (this, e))
>                                        ^
> make[2]: *** [c-family/c-pretty-print.o] Error 1
>

I fixed the build with the obvious r265078:

2018-10-12  Christophe Lyon  <christophe.lyon@linaro.org>

       PR c++/87364
       * c-pretty-print.c (c_pretty_printer::constant): Fix typo.

Index: gcc/c-family/c-pretty-print.c
===================================================================
--- gcc/c-family/c-pretty-print.c       (revision 265077)
+++ gcc/c-family/c-pretty-print.c       (revision 265078)
@@ -1138,7 +1138,7 @@
        else if (type == char_type_node)
          pp_c_character_constant (this, e);
        else if (TREE_CODE (type) == ENUMERAL_TYPE)
-         pp_c_enumeration_constant (this, e))
+         pp_c_enumeration_constant (this, e);
        else
          pp_c_integer_constant (this, e);
       }

Thanks for the commit, for fixing the formatting issues and for writing the
detailed ChangeLog entry.
Sorry for those issues; all looks good now - there are only whitespace
diffs between trunk and my patched version.
My tests pass, including the test code attached to the bug report.

Will

IIRC I cut-n-pasted the patch from terminal to gmail, that must've messed
with formatting.
The patch was also attached to the bug report. I'll attach as text to the
email next time.

On Fri, Oct 12, 2018 at 3:39 AM Christophe Lyon <christophe.lyon@linaro.org>
wrote:

> On Fri, 12 Oct 2018 at 07:47, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
> >
> > On Fri, 12 Oct 2018 at 05:37, Jeff Law <law@redhat.com> wrote:
> > >
> > > On 10/8/18 11:12 AM, will wray wrote:
> > > > Hi,
> > > >
> > > > A PR to fix Bug 87364 - Pretty print of enumerator never prints the
> id,
> > > > always falls back to C-style cast output
> > > >
> > > > The fix is tested with the code attached to the bug report and
> additional
> > > > usage over the past few weeks.
> > > >
> > > > Bootstrap and regtested on x86_64-linux. OK for trunk?
> > > >
> > > >
> > > > 2018-10-08  Will Wray  <wjwray@gmail.com>
> > > >
> > > > PR c++/87364
> > > > * c-pretty-print.c (pp_c_enumeration_constant): fix comparison.
> > > > * cxx-pretty-print.c (pp_cxx_enumeration_constant): New; add
> > > > nested-specifiers prefix to enum id plus enum type for scoped enum.
> > > THanks.  I fixed up some whitespace issues and the ChangeLog entry
> > > before committing to the trunk.
> > >
> > > Also note for any future contributions, it looks like your mailer is
> > > messing up tabs and wrapping lines. That makes applying the patch more
> > > difficult than it should be.
> > >
> >
> > Hi,
> >
> > This commit broke the GCC builds:
> >
> /tmp/3768585_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/c-family/c-pretty-print.c:
> > In member function 'virtual void c_pretty_printer::constant(tree)':
> >
> /tmp/3768585_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/c-family/c-pretty-print.c:1141:39:
> > error: expected ';' before ')' token
> >     pp_c_enumeration_constant (this, e))
> >                                        ^
> > make[2]: *** [c-family/c-pretty-print.o] Error 1
> >
>
> I fixed the build with the obvious r265078:
>
> 2018-10-12  Christophe Lyon  <christophe.lyon@linaro.org>
>
>        PR c++/87364
>        * c-pretty-print.c (c_pretty_printer::constant): Fix typo.
>
> Index: gcc/c-family/c-pretty-print.c
> ===================================================================
> --- gcc/c-family/c-pretty-print.c       (revision 265077)
> +++ gcc/c-family/c-pretty-print.c       (revision 265078)
> @@ -1138,7 +1138,7 @@
>         else if (type == char_type_node)
>           pp_c_character_constant (this, e);
>         else if (TREE_CODE (type) == ENUMERAL_TYPE)
> -         pp_c_enumeration_constant (this, e))
> +         pp_c_enumeration_constant (this, e);
>         else
>           pp_c_integer_constant (this, e);
>        }
>