[cfe-commits] [PATCH] Pretty print identifiers even while suppressing specifiers

[Eli Friedman]

On Tue, Oct 16, 2012 at 5:28 AM, Benoit Perrot <benoit at lrde.epita.fr> wrote:
> Hello all,
>
> On the following input code:
>
>   for (int i = 42, k = 2097; false; )
>     ;
>
> Pretty printing only the init part of the "for" loop, gives the
> following:
>
>   int i = 42, = 2097
>
> My understanding is that, when printing the "VarDecl" of the group
> constituing the said init, the ones following the first are printed
> with a policy asking to suppress specifiers (see "Decl::printGroup").
> This policy reaches "TypePrinter::print()", which simply returns when
> specifiers are suppressed - yet this method is also responsible of
> displaying variable identifiers (kept in "PlaceHolder"):
>
>   void TypePrinter::print(const Type *T, Qualifiers Quals, raw_ostream &OS,
>                           StringRef PlaceHolder) {
>     if (!T) {
>       OS << "NULL TYPE";
>       return;
>     }
>
>     if (Policy.SuppressSpecifiers && T->isSpecifierType())
>       return;
>
>     SaveAndRestore<bool> PHVal(HasEmptyPlaceHolder, PlaceHolder.empty());
>
>     printBefore(T, Quals, OS);
>     OS << PlaceHolder; // < Displays the identifier
>     printAfter(T, Quals, OS);
>   }
>
> The following pretty printers are using "TypePrinter::print", and are
> behaving in a manner that differs from what I was expecting after
> reading the description of "PrintingPolicy::SuppressSpecifiers":
>
> - "void DeclPrinter::VisitTypedefDecl(TypedefDecl *D)"
>
>   Which on:
>     typedef unsigned char foo_t;
>
>   Outputs: (nothing is displayed)
>
>
>   While I was expecting:
>     foo_t
>
> - "void DeclPrinter::VisitFunctionDecl(FunctionDecl *D)"
>
>   On:
>     void bar(int i);
>
>   Outputs: (nothing is displayed - the prototype is entirely
>   built internally - see implementation)
>
>
>   While I was expecting:
>     bar(int i)
>
> - "void DeclPrinter::VisitVarDecl(VarDecl *D)"
>
>   On:
>     int x = 42;
>
>   Outputs: (variable identifier is not displayed)
>      = 42
>
>   While it should have output (see initial problem):
>     x = 42
>
> If I am right, then "TypePrinter::print()" should be written:
>
>   void TypePrinter::print(const Type *T, Qualifiers Quals, raw_ostream &OS,
>                           StringRef PlaceHolder) {
>     if (!T) {
>       OS << "NULL TYPE";
>       return;
>     }
>
>     SaveAndRestore<bool> PHVal(HasEmptyPlaceHolder, PlaceHolder.empty());
>
>     printBefore(T, Quals, OS); // (returns immediately when suppressing
>                                // specifiers, see implementation)
>     OS << PlaceHolder;
>     printAfter(T, Quals, OS);
>   }
>
> This fixes the initial problem (pretty printing variable declarations
> of "for" loops), and gives the results I think was expected on "typedef"
> and function declarations.
>
> Attached herewith is a patch modifying "TypePrinter::print()" accordingly.

Please include a testcase with a patch where appropriate.  (In this
case, see test/SemaCXX/ast-print.cpp .)

-Eli