[PATCH] D30946: [ScopePrinting] Added support to print full scopes of types and declarations.

[Simon Schroeder via Phabricator via cfe-commits]

schroedersi added a comment.

In https://reviews.llvm.org/D30946#740567, @bkramer wrote:

> Also the mutable state in PrintingPolicy is really really ugly, is there no better way for this? :(


Thanks for your comment :-)

I assume with mutable state you mean `PrintingPolicy::TemporarySuppressScope`? I do not really like it either. But other states of the policy are also mutated during the printing process (e.g. SuppressScope, SuppressTagKeyword and SuppressSpecifiers inside the RAII helpers).

A little background: It is necessary that scope printing can be temporarily suppressed without changing the actual scope printing kind setting. For example, in the case of an ElaboratedType, it is necessary that the underlying type is printed without the outer scope (because it has already been printed based on how it was written in the source and based on the scope printing kind setting). However, the internal type should be printed according to the scope printing kind setting. Another example are types inside a nested name specifier. Again, the outer scope can not be printed. However, inner scopes (e.g. for template arguments) must be printed according to the scope printing kind setting.
For this temporal suppression, the information about whether a scope has already been suppressed and thus the scope printing kind setting must be used, must be shared over all member functions involved in the printing. These member functions are the TypePrinter member functions and
`TemplateName::print`, `NestedNameSpecifier::print`, and `NamedDecl::printQualifiedName`. Currently, the only object shared across all these member functions is the PrintingPolicy. That is why I added the state to the PrintingPolicy.

As an alternative to the current solution, the above-mentioned member functions could each be extended by a "PrintingContext" argument, which then contains the variable states.

And a bit of clarification: Without the patch, even with `SuppressScope = true` it is not possible to suppress all scopes, and inner types are printed with a scope most of the time (which seems to lead to the desired effect for most users). For example, printing the type of `A` in `namespace N{class B{}; template<typename T> class C{}; C<::N::B> A;}` with `SuppressScope = true` results in `C< ::N::B>` which still includes scopes. That is, there is already some kind of temporary suppress scope functionality, but it is called SuppressScope. The patch only expands this functionality to full scopes (see summary above), makes the behavior more consistent, and ensures that the settings do what - at least I - would expect from their name and documentation.

I hope this explains my decision. Either way, I look forward to further comments :-).


https://reviews.llvm.org/D30946