Pretty print of C / C++ enumerations

Pretty print of C / C++ enumerations

[will wray]

There's a bug in gcc pretty printing of C / C++ enumerators:

  GCC should print the id of an enumerated value
  but prints a C-style cast (the fallback for non-enumerated values)

The bug shows up in __PRETTY_FUNCTION__ output, for instance.
You can see it in the error output of this Compiler Explorer example
https://godbolt.org/z/1df0Sk

  enum e { a, b, c=0 };
  template <auto> struct wauto; // deliberately incomplete to trigger..
  wauto<a> v; // error: aggregate 'wauto<(e)0> v' has incomplete type..

The error output should print 'a' in place of '(e)0'

(Clang gets it right with 'wauto<a>',
 MSVC wrongly prints 'wauto<0>' so also loses type information.)

This mail shows a one-line hack to fix the bug... almost...
I'd like advice or assistance towards a proper fix.

The function is ~30 lines of code so I paste it after an intro.

The intent of the code is to print, for a given integral constant:

   - The enum id of the first enumerator with that value, or
   - a C-style cast if no enumerator is found with that value.

In  c-pretty-print.c
    c_pretty_printer::constant calls pp_c_enumeration_constant

/******************** pp_c_enumeration_constant ****************/

/* 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. */

static bool
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 = TREE_CHAIN (value))
    ;

  if (value != NULL_TREE)
    pp->id_expression (TREE_PURPOSE (value));
  else
  {
    /* Value must have been cast. */
    pp_c_type_cast (pp, type);
    value_is_named = false;
  }

  return value_is_named;
}
/***************************************************************/
The code iterates over the enumerators, comparing with the given value,
prints the id if found else falls back to print the C-style cast.
However, the comparison always fails so it always prints the cast
(after iterating over all the enumerators) and never the id.

The comparison in the enumeration loop can be fixed by adding DECL_INITIAL
to 'unwrap' the enumerator's value so it compares correctly as INTEGER_CST,
terminating the loop and pretty-printing the id instead of the C-style cast.

  /* 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))
     ;

With this patch, the loop exits early when it finds an enumerator of the
given value and prints the found-enumerator's id via:

  if (value != NULL_TREE)
    pp->id_expression (TREE_PURPOSE (value));

However...
This code is located in c-pretty-print.c
and has not been updated to deal with C++11 scoped enums.

I can share a more complete fix that duplicates c-pretty-print.c
code to cxx-pretty-print.c and modifies it for scoped enums.
I'm learning as I hack but would like guidance on a proper fix
from anyone more familiar with gcc pretty print and/or grammar -
the guideline comment, at the top of the file, states:

/* The pretty-printer code is primarily designed to closely follow
   (GNU) C and C++ grammars... */

Another thought.
The pretty-printing code will need an overhaul for C++20 with
the proposed changes for generalised non-type template args.

Thanks for any pointers.

Re: Pretty print of C / C++ enumerations

[will wray]

Also, the docs need a fix.
The GCC Internals doc is inconsistent on enumerator value.

GCC Internals https://gcc.gnu.org/onlinedocs/gccint.pdf
Chapter 11 GENERIC 11.3 Types (p 158 of current pdf):

  ENUMERAL_TYPE
    Used to represent an enumeration type
    ... the TREE_VALUE will be an INTEGER_CST giving the value
     assigned to that constant...

 *This is incorrect - TREE_VALUE is not an INTEGER_CST -
  this is the bug in gcc enumerator pretty print.*

Chapter 11 GENERIC  11.4 Declarations (p 161)

  CONST_DECL
    These nodes are used to represent enumeration constants.
    The value of the constant is given by DECL_INITIAL which
    will be an INTEGER_CST with the same type as the TREE_TYPE
    of the CONST_DECL, i.e., an ENUMERAL_TYPE.

 *This is correct - DECL_INITIAL is needed to get the value*

My guess is that DECL_INITIAL was introduced at some point
but not all the code and docs were updated for the change.


On Tue, Sep 18, 2018 at 3:58 PM will wray <wjwray@gmail.com> wrote:

> There's a bug in gcc pretty printing of C / C++ enumerators:
>
>   GCC should print the id of an enumerated value
>   but prints a C-style cast (the fallback for non-enumerated values)
>
> The bug shows up in __PRETTY_FUNCTION__ output, for instance.
> You can see it in the error output of this Compiler Explorer example
> https://godbolt.org/z/1df0Sk
>
>   enum e { a, b, c=0 };
>   template <auto> struct wauto; // deliberately incomplete to trigger..
>   wauto<a> v; // error: aggregate 'wauto<(e)0> v' has incomplete type..
>
> The error output should print 'a' in place of '(e)0'
>
> (Clang gets it right with 'wauto<a>',
>  MSVC wrongly prints 'wauto<0>' so also loses type information.)
>
> This mail shows a one-line hack to fix the bug... almost...
> I'd like advice or assistance towards a proper fix.
>
> The function is ~30 lines of code so I paste it after an intro.
>
> The intent of the code is to print, for a given integral constant:
>
>    - The enum id of the first enumerator with that value, or
>    - a C-style cast if no enumerator is found with that value.
>
> In  c-pretty-print.c
>     c_pretty_printer::constant calls pp_c_enumeration_constant
>
> /******************** pp_c_enumeration_constant ****************/
>
> /* 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. */
>
> static bool
> 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 = TREE_CHAIN (value))
>     ;
>
>   if (value != NULL_TREE)
>     pp->id_expression (TREE_PURPOSE (value));
>   else
>   {
>     /* Value must have been cast. */
>     pp_c_type_cast (pp, type);
>     value_is_named = false;
>   }
>
>   return value_is_named;
> }
> /***************************************************************/
> The code iterates over the enumerators, comparing with the given value,
> prints the id if found else falls back to print the C-style cast.
> However, the comparison always fails so it always prints the cast
> (after iterating over all the enumerators) and never the id.
>
> The comparison in the enumeration loop can be fixed by adding DECL_INITIAL
> to 'unwrap' the enumerator's value so it compares correctly as INTEGER_CST,
> terminating the loop and pretty-printing the id instead of the C-style
> cast.
>
>   /* 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))
>      ;
>
> With this patch, the loop exits early when it finds an enumerator of the
> given value and prints the found-enumerator's id via:
>
>   if (value != NULL_TREE)
>     pp->id_expression (TREE_PURPOSE (value));
>
> However...
> This code is located in c-pretty-print.c
> and has not been updated to deal with C++11 scoped enums.
>
> I can share a more complete fix that duplicates c-pretty-print.c
> code to cxx-pretty-print.c and modifies it for scoped enums.
> I'm learning as I hack but would like guidance on a proper fix
> from anyone more familiar with gcc pretty print and/or grammar -
> the guideline comment, at the top of the file, states:
>
> /* The pretty-printer code is primarily designed to closely follow
>    (GNU) C and C++ grammars... */
>
> Another thought.
> The pretty-printing code will need an overhaul for C++20 with
> the proposed changes for generalised non-type template args.
>
> Thanks for any pointers.
>
>

Re: Pretty print of C / C++ enumerations

[Jonathan Wakely]

This is not the right list. Bug reports should go to bugzilla, and
discussion of changes/fixes to GCC code should go to the gcc mailing
list.
On Wed, 19 Sep 2018 at 13:49, will wray <wjwray@gmail.com> wrote:
>
> Also, the docs need a fix.
> The GCC Internals doc is inconsistent on enumerator value.
>
> GCC Internals https://gcc.gnu.org/onlinedocs/gccint.pdf
> Chapter 11 GENERIC 11.3 Types (p 158 of current pdf):
>
>   ENUMERAL_TYPE
>     Used to represent an enumeration type
>     ... the TREE_VALUE will be an INTEGER_CST giving the value
>      assigned to that constant...
>
>  *This is incorrect - TREE_VALUE is not an INTEGER_CST -
>   this is the bug in gcc enumerator pretty print.*
>
> Chapter 11 GENERIC  11.4 Declarations (p 161)
>
>   CONST_DECL
>     These nodes are used to represent enumeration constants.
>     The value of the constant is given by DECL_INITIAL which
>     will be an INTEGER_CST with the same type as the TREE_TYPE
>     of the CONST_DECL, i.e., an ENUMERAL_TYPE.
>
>  *This is correct - DECL_INITIAL is needed to get the value*
>
> My guess is that DECL_INITIAL was introduced at some point
> but not all the code and docs were updated for the change.
>
>
> On Tue, Sep 18, 2018 at 3:58 PM will wray <wjwray@gmail.com> wrote:
>
> > There's a bug in gcc pretty printing of C / C++ enumerators:
> >
> >   GCC should print the id of an enumerated value
> >   but prints a C-style cast (the fallback for non-enumerated values)
> >
> > The bug shows up in __PRETTY_FUNCTION__ output, for instance.
> > You can see it in the error output of this Compiler Explorer example
> > https://godbolt.org/z/1df0Sk
> >
> >   enum e { a, b, c=0 };
> >   template <auto> struct wauto; // deliberately incomplete to trigger..
> >   wauto<a> v; // error: aggregate 'wauto<(e)0> v' has incomplete type..
> >
> > The error output should print 'a' in place of '(e)0'
> >
> > (Clang gets it right with 'wauto<a>',
> >  MSVC wrongly prints 'wauto<0>' so also loses type information.)
> >
> > This mail shows a one-line hack to fix the bug... almost...
> > I'd like advice or assistance towards a proper fix.
> >
> > The function is ~30 lines of code so I paste it after an intro.
> >
> > The intent of the code is to print, for a given integral constant:
> >
> >    - The enum id of the first enumerator with that value, or
> >    - a C-style cast if no enumerator is found with that value.
> >
> > In  c-pretty-print.c
> >     c_pretty_printer::constant calls pp_c_enumeration_constant
> >
> > /******************** pp_c_enumeration_constant ****************/
> >
> > /* 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. */
> >
> > static bool
> > 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 = TREE_CHAIN (value))
> >     ;
> >
> >   if (value != NULL_TREE)
> >     pp->id_expression (TREE_PURPOSE (value));
> >   else
> >   {
> >     /* Value must have been cast. */
> >     pp_c_type_cast (pp, type);
> >     value_is_named = false;
> >   }
> >
> >   return value_is_named;
> > }
> > /***************************************************************/
> > The code iterates over the enumerators, comparing with the given value,
> > prints the id if found else falls back to print the C-style cast.
> > However, the comparison always fails so it always prints the cast
> > (after iterating over all the enumerators) and never the id.
> >
> > The comparison in the enumeration loop can be fixed by adding DECL_INITIAL
> > to 'unwrap' the enumerator's value so it compares correctly as INTEGER_CST,
> > terminating the loop and pretty-printing the id instead of the C-style
> > cast.
> >
> >   /* 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))
> >      ;
> >
> > With this patch, the loop exits early when it finds an enumerator of the
> > given value and prints the found-enumerator's id via:
> >
> >   if (value != NULL_TREE)
> >     pp->id_expression (TREE_PURPOSE (value));
> >
> > However...
> > This code is located in c-pretty-print.c
> > and has not been updated to deal with C++11 scoped enums.
> >
> > I can share a more complete fix that duplicates c-pretty-print.c
> > code to cxx-pretty-print.c and modifies it for scoped enums.
> > I'm learning as I hack but would like guidance on a proper fix
> > from anyone more familiar with gcc pretty print and/or grammar -
> > the guideline comment, at the top of the file, states:
> >
> > /* The pretty-printer code is primarily designed to closely follow
> >    (GNU) C and C++ grammars... */
> >
> > Another thought.
> > The pretty-printing code will need an overhaul for C++20 with
> > the proposed changes for generalised non-type template args.
> >
> > Thanks for any pointers.
> >
> >